-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
refactor(ui): extract shared task form components for consistent moda… #765
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…l sizing Create shared components to unify TaskCreationWizard, TaskEditDialog, and TaskDetailModal with consistent full-height modal sizing. New shared components in task-form/: - TaskModalLayout: Full-height modal matching TaskDetailModal (95vw, max-w-5xl) - TaskFormFields: Common form fields (description, title, profile, classification) - ClassificationFields: Task classification 2x2 grid dropdowns - useImageUpload: Hook for image paste/drop handling Benefits: - All 3 task modals now have identical dimensions and positioning - Reduced code duplication (1,938 → 1,651 lines total) - TaskCreationWizard: 1,176 → 623 lines (47% reduction) - TaskEditDialog: 762 → 293 lines (62% reduction) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
📝 WalkthroughWalkthroughRefactors task create/edit UIs into a modular modal system (TaskModalLayout + TaskFormFields + ClassificationFields + useImageUpload), adds FileAutocomplete and TaskFileExplorerDrawer, introduces draft persistence/restoration, and adds project-default Git branch + baseBranch selector; centralizes image handling into a shared hook. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Wizard as TaskCreationWizard
participant Modal as TaskModalLayout
participant Form as TaskFormFields
participant Auto as FileAutocomplete
participant Draft as DraftStore
participant API as createTask API
User->>Wizard: Open wizard
Wizard->>Draft: Load draft on open
alt draft exists
Draft-->>Wizard: Return draft
Wizard->>Modal: Open with restored data
else
Wizard->>Modal: Open empty
end
Modal->>Form: Inject props & handlers
User->>Form: Type description with "@mention"
Form->>Auto: Request suggestions
Auto-->>Form: Insert selected file mention
User->>Form: Paste/Drop images
Form->>Form: useImageUpload validates & adds thumbnails
User->>Form: Select base branch
User->>Modal: Click Create
Modal->>Wizard: handleCreate(payload)
Wizard->>API: createTask(metadata, images)
alt success
Wizard->>Draft: Clear draft
Wizard-->>User: Close modal
else
API-->>Wizard: Error
Wizard->>Form: Show error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)apps/frontend/src/**/*.{ts,tsx,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/frontend/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/frontend/**/*.{ts,tsx}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-12-30T16:38:36.314ZApplied to files:
📚 Learning: 2026-01-04T23:59:45.209ZApplied to files:
🧬 Code graph analysis (1)apps/frontend/src/renderer/components/TaskEditDialog.tsx (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @AndyMik90, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the user interface for task creation and editing by extracting common components and logic into reusable modules. The primary goal is to achieve consistent modal sizing and behavior across Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request is an excellent refactoring that significantly improves the structure and maintainability of the task creation and editing modals. By extracting shared logic into TaskModalLayout, TaskFormFields, ClassificationFields, and the useImageUpload hook, you've greatly reduced code duplication and made the TaskCreationWizard and TaskEditDialog components much cleaner and easier to understand. The new components are well-designed and reusable.
I've found one issue with how the descriptionRef is handled in the new TaskFormFields component, which would break the @mention autocomplete feature. My review comment includes details on how to fix it.
Overall, this is a high-quality contribution that makes the UI codebase more robust and scalable. Great work!
| children | ||
| }: TaskFormFieldsProps) { | ||
| const { t } = useTranslation('tasks'); | ||
| const descriptionRef = useRef<HTMLTextAreaElement>(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The descriptionRef is created locally within TaskFormFields, but the parent component TaskCreationWizard needs access to this ref to correctly position the @mention autocomplete popup. Currently, the parent's ref is disconnected from the actual <Textarea> element, which will break the autocomplete positioning.
To fix this, you should accept the descriptionRef as a prop and forward it to the <Textarea>. This involves:
- Adding
descriptionRef: React.RefObject<HTMLTextAreaElement>;toTaskFormFieldsProps. - Destructuring
descriptionReffrom the component's props. - Removing this local
useRefcall. - Passing the
descriptionRefprop to therefof the<Textarea>component on line 158.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/frontend/src/renderer/components/TaskEditDialog.tsx (2)
201-205: Logic issue:isAutoProfileis always set totruewhen phase configs exist.The
isAutoProfilemetadata is set unconditionally whenphaseModels && phaseThinkingare present (line 201-205), but this applies even when the user selects a non-auto profile like "custom" or a specific preset. This could cause incorrect profile detection when editing the task later.🔎 Suggested fix
if (model) metadataUpdates.model = model as ModelType; if (thinkingLevel) metadataUpdates.thinkingLevel = thinkingLevel as ThinkingLevel; if (phaseModels && phaseThinking) { - metadataUpdates.isAutoProfile = true; + metadataUpdates.isAutoProfile = profileId === 'auto'; metadataUpdates.phaseModels = phaseModels; metadataUpdates.phaseThinking = phaseThinking; }
228-250: New i18n violations: Hardcoded modal and button strings.Lines 231-232 (modal title/description) and lines 237, 243, 246 (button labels) contain hardcoded user-facing text.
🔎 Suggested fix
+ import { useTranslation } from 'react-i18next'; ... export function TaskEditDialog({ task, open, onOpenChange, onSaved }: TaskEditDialogProps) { + const { t } = useTranslation('tasks'); ... return ( <TaskModalLayout open={open} onOpenChange={onOpenChange} - title="Edit Task" - description="Update task details including title, description, classification, images, and settings. Changes will be saved to the spec files." + title={t('edit.title')} + description={t('edit.description')} disabled={isSaving} footer={ <div className="flex items-center justify-end gap-3"> - <Button variant="outline" onClick={() => onOpenChange(false)} disabled={isSaving}> - Cancel + <Button variant="outline" onClick={() => onOpenChange(false)} disabled={isSaving}> + {t('common.cancel')} </Button> <Button onClick={handleSave} disabled={isSaving || !isValid}> {isSaving ? ( <> <Loader2 className="mr-2 h-4 w-4 animate-spin" /> - Saving... + {t('common.saving')} </> ) : ( - 'Save Changes' + t('common.saveChanges') )} </Button> </div> } >apps/frontend/src/renderer/components/TaskCreationWizard.tsx (2)
335-339: Same logic issue:isAutoProfileunconditionally set totrue.Identical to the issue in TaskEditDialog -
isAutoProfileshould reflect whether the user actually selected the "auto" profile, not just whether phase configs exist.🔎 Suggested fix
if (model) metadata.model = model; if (thinkingLevel) metadata.thinkingLevel = thinkingLevel; if (phaseModels && phaseThinking) { - metadata.isAutoProfile = true; + metadata.isAutoProfile = profileId === 'auto'; metadata.phaseModels = phaseModels; metadata.phaseThinking = phaseThinking; }
433-502: Multiple new i18n violations in modal and footer.This section contains numerous hardcoded user-facing strings that should use translation keys:
- Line 437: "Create New Task"
- Line 438: Description text
- Lines 456-458: "Draft restored"
- Lines 465-466: "Start Fresh"
- Lines 482: "Hide Files" / "Browse Files"
- Lines 488-499: "Cancel", "Creating...", "Create Task"
🔎 Suggested approach
Since
useTranslationis already imported (line 14) and used (line 55), add translation keys for these strings:<TaskModalLayout open={open} onOpenChange={handleClose} - title="Create New Task" - description="Describe what you want to build. The AI will analyze your request and create a detailed specification." + title={t('create.title')} + description={t('create.description')} ...Similar changes needed for all button labels and status messages.
🤖 Fix all issues with AI agents
In @apps/frontend/src/renderer/components/task-form/ClassificationFields.tsx:
- Around line 64-158: Replace all hardcoded user-facing strings in
ClassificationFields.tsx with i18n keys: swap Label text ("Category",
"Priority", "Complexity", "Impact"), SelectValue placeholders ("Select
category", "Select priority", "Select complexity", "Select impact") and the
helper paragraph ("These labels help organize...") for translation lookups
(e.g., t('task.category'), t('task.selectCategory'), t('task.helper') or your
repo's key naming). Import and use the project's translation helper (e.g.,
useTranslation or t) at the top of the component and call it where Label,
SelectValue and the paragraph currently use literal strings; ensure you add
consistent translation keys (task.category, task.selectCategory, task.priority,
task.selectPriority, task.complexity, task.selectComplexity, task.impact,
task.selectImpact, task.labelsHelper) to the locale files.
In @apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx:
- Around line 127-129: The declared descriptionRef in TaskFormFields is unused;
either remove its declaration or actually attach it to the Textarea so it’s not
dead code. If the intent is to control focus from this component, attach
descriptionRef to the Textarea via the ref prop (or convert Textarea to
forwardRef if needed) and update any consuming code to use the same ref;
otherwise simply delete the line that creates descriptionRef to clean up the
unused variable.
- Around line 228-233: The success message is hardcoded in TaskFormFields.tsx
where pasteSuccess controls the UI; replace the literal "Image added
successfully!" with a translated string (e.g., using the existing i18n
hook/utility used in this component such as t('...') or useTranslation()) so it
uses a translation key like "taskForm.imageAddedSuccess"; update the JSX that
renders when pasteSuccess is true to call the translation function and pass any
needed interpolation instead of the hardcoded text.
- Around line 235-250: The TaskFormFields component contains hardcoded user
strings in the Label, Input placeholder, and the explanatory paragraph; replace
those literals with i18n translation keys by importing and using your
translation hook/function (e.g., useTranslation/t) inside TaskFormFields and
call it where Label, Input and the paragraph render (refer to the Label, Input,
and the surrounding paragraph elements and keep existing props like
id={`${prefix}title`}, value={title}, onChange={onTitleChange} and disabled).
Add descriptive keys such as taskForm.title.label, taskForm.title.optional,
taskForm.title.placeholder and taskForm.title.help (or your project’s key naming
convention) and use t(...) for each string so the UI reads from the translations
instead of hardcoded text.
In @apps/frontend/src/renderer/components/task-form/useImageUpload.ts:
- Around line 136-144: The setTimeout used to reset setPasteSuccess in the
useImageUpload hook can fire after the component unmounts and cause state
updates on an unmounted component; modify the hook to track mounted status or
store the timeout id and clear it on cleanup: create a mounted ref (e.g.,
isMountedRef) set true on mount and false on unmount (or store timeoutId and
call clearTimeout in cleanup), check isMountedRef.current before calling
setPasteSuccess(false) (or clear the timeout in the hook's cleanup), and ensure
this logic is wired into the same scope where setPasteSuccess is invoked inside
the paste handling code in useImageUpload so no state update occurs after
unmount.
In @apps/frontend/src/renderer/components/TaskCreationWizard.tsx:
- Around line 559-618: The Git options UI uses hardcoded English strings;
replace them with i18n lookups by importing and using the project's translation
hook (e.g., useTranslation / useTranslations -> t) at the top of
TaskCreationWizard and swap the literal texts inside the button (where
setShowGitOptions/showGitOptions is used), the Label for "Base Branch
(optional)", the SelectValue placeholder and the SelectItem content that
currently render "Use project default", and the help paragraph text; create
appropriate translation keys (e.g., taskCreation.gitOptions.title,
taskCreation.gitOptions.baseBranchLabel,
taskCreation.gitOptions.useProjectDefault, taskCreation.gitOptions.help) and
pass the runtime baseBranch/projectDefaultBranch values into the translation
placeholders where needed (for the badge and placeholder).
- Around line 249-253: The hardcoded charWidth = 8 in TaskCreationWizard (used
with lines[currentLineIndex], currentLineIndex, paddingTop, lineHeight,
paddingLeft and rect.width - 300) can misplace the autocomplete; replace the
magic constant with a real measurement—either compute the per-character width
using a canvas 2D context and measureText for the component's current font or
integrate a caret-position utility (e.g.,
textarea-caret-position/textarea-caret) to get the exact caret coordinates; then
use that measured horizontal offset instead of currentLineLength * charWidth
when computing left so autocomplete aligns reliably across fonts and sizes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
apps/frontend/src/renderer/components/TaskCreationWizard.tsxapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/task-form/TaskModalLayout.tsxapps/frontend/src/renderer/components/task-form/index.tsapps/frontend/src/renderer/components/task-form/useImageUpload.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/renderer/components/task-form/index.tsapps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/TaskCreationWizard.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/renderer/components/task-form/index.tsapps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/TaskCreationWizard.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/renderer/components/task-form/index.tsapps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/TaskCreationWizard.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/TaskCreationWizard.tsx
🧬 Code graph analysis (4)
apps/frontend/src/renderer/components/task-form/ClassificationFields.tsx (2)
apps/frontend/src/shared/types/task.ts (4)
TaskCategory(160-169)TaskPriority(156-156)TaskComplexity(154-154)TaskImpact(155-155)apps/frontend/src/shared/constants/task.ts (4)
TASK_CATEGORY_LABELS(100-110)TASK_PRIORITY_LABELS(166-171)TASK_COMPLEXITY_LABELS(128-134)TASK_IMPACT_LABELS(148-153)
apps/frontend/src/renderer/components/TaskEditDialog.tsx (2)
apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx (1)
TaskModalLayout(39-136)apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx (1)
TaskFormFields(90-336)
apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx (5)
apps/frontend/src/shared/types/task.ts (6)
ModelType(159-159)TaskCategory(160-169)TaskPriority(156-156)TaskComplexity(154-154)TaskImpact(155-155)ImageAttachment(113-121)apps/frontend/src/renderer/components/task-form/useImageUpload.ts (1)
useImageUpload(53-243).design-system/src/components/Input.tsx (1)
Input(4-24)apps/frontend/src/renderer/components/AgentProfileSelector.tsx (1)
AgentProfileSelector(72-380)apps/frontend/src/renderer/components/task-form/ClassificationFields.tsx (1)
ClassificationFields(46-161)
apps/frontend/src/renderer/components/TaskCreationWizard.tsx (4)
apps/frontend/src/renderer/stores/task-store.ts (3)
isDraftEmpty(577-588)saveDraft(517-533)clearDraft(557-564)apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx (1)
TaskModalLayout(39-136)apps/frontend/src/renderer/components/task-form/index.ts (2)
TaskModalLayout(11-11)TaskFormFields(12-12)apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx (1)
TaskFormFields(90-336)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (2)
apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx (1)
39-136: LGTM! Well-structured reusable modal layout.The component properly composes Radix UI Dialog primitives with good accessibility support (Title, Description, sr-only close label). The
disabledprop correctly prevents closing during loading states, and the optional sidebar pattern is cleanly implemented.apps/frontend/src/renderer/components/task-form/index.ts (1)
1-14: LGTM!Clean barrel export module with good documentation. Centralizes the task-form component exports appropriately.
apps/frontend/src/renderer/components/task-form/ClassificationFields.tsx
Show resolved
Hide resolved
apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx
Outdated
Show resolved
Hide resolved
| const currentLineLength = lines[currentLineIndex].length; | ||
|
|
||
| const charWidth = 8; | ||
| const top = paddingTop + (currentLineIndex + 1) * lineHeight + 4; | ||
| const left = paddingLeft + Math.min(currentLineLength * charWidth, rect.width - 300); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Hardcoded charWidth may cause positioning issues.
The charWidth = 8 assumption for autocomplete positioning won't match all font configurations. Consider measuring actual character width or using a more robust positioning approach.
🔎 More robust approach
- const charWidth = 8;
- const top = paddingTop + (currentLineIndex + 1) * lineHeight + 4;
- const left = paddingLeft + Math.min(currentLineLength * charWidth, rect.width - 300);
+ // Create a temporary span to measure actual text width
+ const measureSpan = document.createElement('span');
+ measureSpan.style.cssText = `
+ position: absolute;
+ visibility: hidden;
+ white-space: pre;
+ font: ${textareaStyle.font};
+ `;
+ measureSpan.textContent = lines[currentLineIndex];
+ document.body.appendChild(measureSpan);
+ const textWidth = measureSpan.offsetWidth;
+ document.body.removeChild(measureSpan);
+
+ const top = paddingTop + (currentLineIndex + 1) * lineHeight + 4;
+ const left = paddingLeft + Math.min(textWidth, rect.width - 300);Alternatively, consider using a library like textarea-caret for accurate caret positioning.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/TaskCreationWizard.tsx around lines
249 - 253, The hardcoded charWidth = 8 in TaskCreationWizard (used with
lines[currentLineIndex], currentLineIndex, paddingTop, lineHeight, paddingLeft
and rect.width - 300) can misplace the autocomplete; replace the magic constant
with a real measurement—either compute the per-character width using a canvas 2D
context and measureText for the component's current font or integrate a
caret-position utility (e.g., textarea-caret-position/textarea-caret) to get the
exact caret coordinates; then use that measured horizontal offset instead of
currentLineLength * charWidth when computing left so autocomplete aligns
reliably across fonts and sizes.
|
|
||
| await processImageItems(imageFiles, { isFromPaste: false }); | ||
| }, | ||
| [disabled, processImageItems] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File reference drop from explorer no longer works
Medium Severity
The handleDrop function in useImageUpload only processes image files. The original TaskCreationWizard had logic to check for file reference drops from the file explorer by reading e.dataTransfer.getData('application/json') and looking for type === 'file-reference' to insert @mentions at the cursor position. This special handling was removed during the refactoring, so dragging files from the TaskFileExplorerDrawer sidebar into the description textarea no longer inserts @mentions.
| } | ||
|
|
||
| if (newImages.length > 0) { | ||
| onImagesChange([...images, ...newImages]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale closure in image operations causes race condition
Medium Severity
The old code used functional state updates (setImages(prev => [...prev, ...newImages])) which always reference the latest state. The new useImageUpload hook uses images from the closure (onImagesChange([...images, ...newImages]) and images.filter(...)). During rapid operations (quick successive pastes or removes), the closure's images value can be stale, causing race conditions where images are lost or removed images reappear because the update overwrites concurrent changes.
Additional Locations (1)
AndyMik90
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 Auto Claude PR Review
Merge Verdict: 🔴 BLOCKED
Blocked: PR has merge conflicts with base branch. Resolve conflicts before merge.
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | High | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
🚨 Blocking Issues (Must Fix)
- Merge Conflicts: PR has conflicts with base branch that must be resolved
Findings Summary
- High: 1 issue(s)
- Medium: 4 issue(s)
- Low: 2 issue(s)
Generated by Auto Claude PR Review
Findings (7 selected of 7 total)
🟠 [c3b68acc2e5e] [HIGH] Disconnected descriptionRef breaks @ mention autocomplete feature
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:111
The descriptionRef created at line 111 in TaskCreationWizard is never passed to TaskFormFields. TaskFormFields creates its own separate local ref at line 128 which is attached to the Textarea. The parent's descriptionRef.current will always be null, causing: (1) handleDescriptionChange to use cursorPos=0 instead of actual cursor position, (2) autocomplete positioning calculation to fail silently since textarea is null, (3) handleAutocompleteSelect to return early without setting focus/cursor position. This completely breaks the @ mention autocomplete feature for file references.
Suggested fix:
Pass descriptionRef from TaskCreationWizard to TaskFormFields as a prop, or use React.forwardRef to expose the textarea ref. Add a new prop to TaskFormFieldsProps: `descriptionRef?: React.RefObject<HTMLTextAreaElement>` and use it in TaskFormFields instead of creating a local ref.
🟡 [3a70e35347c1] [MEDIUM] New file violates i18n requirements - multiple hardcoded strings
📁 apps/frontend/src/renderer/components/task-form/ClassificationFields.tsx:1
This new file introduces 10+ hardcoded user-facing strings that violate the project's mandatory i18n requirement stated in CLAUDE.md: 'CRITICAL: Always use i18n translation keys for all user-facing text in the frontend.' Affected strings: 'Category', 'Priority', 'Complexity', 'Impact', 'Select category/priority/complexity/impact' placeholders, and the help text 'These labels help organize and prioritize tasks...'
Suggested fix:
Add useTranslation hook and replace all hardcoded strings with translation keys from the tasks namespace. Add corresponding keys to apps/frontend/src/shared/i18n/locales/en/tasks.json and fr/tasks.json.
🟡 [e4941ab4e947] [MEDIUM] Multiple hardcoded strings violate i18n requirements
📁 apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx:152
TaskFormFields contains 8+ hardcoded user-facing strings that violate the project's mandatory i18n requirement. While the component imports useTranslation, most text is hardcoded: 'Description', 'Task Title', '(optional)', placeholders, 'Classification (optional)', 'Require human review before coding', and 'Image added successfully!'
Suggested fix:
Replace all hardcoded strings with translation keys. The component already has useTranslation imported - use t('form.description'), t('form.taskTitle'), t('form.classificationOptional'), t('form.requireReviewLabel'), t('images.addedSuccessfully'), etc.
🟡 [7f7d2f5000e9] [MEDIUM] Hardcoded screen reader text 'Close' violates i18n
📁 apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx:106
TaskModalLayout has a hardcoded 'Close' string for screen reader accessibility. While less visible to users, this still violates the project's i18n requirements and should use a translation key for proper localization.
Suggested fix:
Add useTranslation hook and replace with: <span className="sr-only">{t('common:close')}</span>
🟡 [05e5ed3169f5] [MEDIUM] Git options section has hardcoded strings violating i18n
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:573
The Git options section unique to TaskCreationWizard contains hardcoded strings: 'Git Options (optional)', 'Base Branch (optional)', 'Use project default', and the help text explaining branch override functionality.
Suggested fix:
Replace with translation keys: t('gitOptions.title'), t('gitOptions.baseBranch'), t('gitOptions.useProjectDefault'), t('gitOptions.helpText')
🔵 [10fd851c1383] [LOW] Hook file placement inconsistent with codebase pattern
📁 apps/frontend/src/renderer/components/task-form/useImageUpload.ts:1
The codebase has an established pattern of placing hooks in a 'hooks/' subdirectory within component folders (e.g., task-detail/hooks/useTaskDetail.ts, changelog/hooks/useImageUpload.ts). The new useImageUpload.ts is placed directly in the task-form/ directory instead of task-form/hooks/.
Suggested fix:
Move to task-form/hooks/useImageUpload.ts and update imports in TaskFormFields.tsx and index.ts
🔵 [e2eba72e919f] [LOW] setTimeout for success indicator not cleaned up on unmount
📁 apps/frontend/src/renderer/components/task-form/useImageUpload.ts:139
The setTimeout at line 139-140 sets pasteSuccess to false after 2 seconds, but this timeout is never cleared if the component unmounts. While React 18+ doesn't warn about this, it's a code quality issue that could be cleaned up.
Suggested fix:
Store timeout ID and clear on cleanup, or use a useEffect with pasteSuccess dependency to manage the timer lifecycle
This review was generated by Auto Claude.
Resolved conflict in TaskCreationWizard.tsx by keeping the refactored version that uses TaskModalLayout and TaskFormFields components. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/frontend/src/renderer/components/TaskCreationWizard.tsx (2)
156-163: Consider refactoring to avoid eslint-disable.The
eslint-disable-next-line react-hooks/exhaustive-depssuppresses a valid warning.fetchBranchesandfetchProjectDefaultBranchare defined outside the effect but called inside it.♻️ Recommended refactor: move functions inside useEffect or use useCallback
// Fetch branches when dialog opens useEffect(() => { + const fetchBranches = async () => { + if (!projectPath) return; + setIsLoadingBranches(true); + try { + const result = await window.electronAPI.getGitBranches(projectPath); + if (result.success && result.data) { + setBranches(result.data); + } + } catch (err) { + console.error('Failed to fetch branches:', err); + } finally { + setIsLoadingBranches(false); + } + }; + + const fetchProjectDefaultBranch = async () => { + if (!projectId) return; + try { + const result = await window.electronAPI.getProjectEnv(projectId); + if (result.success && result.data?.defaultBranch) { + setProjectDefaultBranch(result.data.defaultBranch); + } else if (projectPath) { + const detectResult = await window.electronAPI.detectMainBranch(projectPath); + if (detectResult.success && detectResult.data) { + setProjectDefaultBranch(detectResult.data); + } + } + } catch (err) { + console.error('Failed to fetch project default branch:', err); + } + }; + if (open && projectPath) { fetchBranches(); fetchProjectDefaultBranch(); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [open, projectPath]); + }, [open, projectPath, projectId]);
74-76: Remove unuseduseWorktreestate variable.The
useWorktreestate is initialized totrue(line 76) and only reset totruein theresetForm()function (line 380). Since it's never set tofalse, the conditional at line 347 (if (!useWorktree) metadata.useWorktree = false;) is dead code that never executes. Either implement the intended UI toggle to allow toggling this state, or remove the unused state and conditional logic.
🤖 Fix all issues with AI agents
In @apps/frontend/src/renderer/components/TaskCreationWizard.tsx:
- Around line 551-562: autocomplete.position is computed from descriptionRef but
descriptionRef is never attached to the textarea in TaskFormFields, so the
FileAutocomplete popup will be mispositioned or error; fix this by wiring the
ref into TaskFormFields: accept a ref (or a prop like descriptionRef) and attach
it to the textarea element (use React.forwardRef if TaskFormFields is a
component), ensure the parent passes the same descriptionRef used to compute
autocomplete.position, and add a null-guard where the position is computed so
you don’t call getBoundingClientRect on a missing ref; this ensures
FileAutocomplete receives a correct position and avoids runtime errors.
- Line 113: TaskCreationWizard declares descriptionRef but never passes it into
TaskFormFields, so descriptionRef.current stays null and the autocomplete
positioning (which reads textarea.getBoundingClientRect() and computed styles)
fails; fix by adding a prop to TaskFormFields (e.g., descriptionRef?:
React.RefObject<HTMLTextAreaElement>) and forwarding that ref to the internal
textarea element inside TaskFormFields, then pass the existing descriptionRef
from TaskCreationWizard into <TaskFormFields descriptionRef={descriptionRef}
.../>; also add a null check before using descriptionRef.current in the
autocomplete positioning code (or alternatively implement an
onDescriptionCursorChange callback prop if you prefer callback approach).
- Around line 424-430: The span in TaskCreationWizard that renders @mentions
currently sets color via both className ("text-info-foreground") and an inline
style (style={{ color: 'hsl(var(--info))' }); remove the redundant inline style
and rely on the CSS utility class for color consistency (or vice versa if you
prefer inline styles); update the span (the element with key={i} and
className="bg-info/20 text-info-foreground rounded px-0.5") to use only one
color approach.
- Around line 442-443: Replace the hardcoded user-facing strings in the
TaskCreationWizard component with i18n translation keys: import and use the
existing i18n hook (useTranslation) in TaskCreationWizard and swap each literal
(e.g., "Create New Task", "Describe what you want to build...", "Draft
restored", "Start Fresh", "Hide Files", "Browse Files", "Cancel", "Creating...",
"Create Task", "Git Options (optional)", "Base Branch (optional)", the
base-branch help text, "Please provide a description", "Failed to create
task...") for t('taskCreation.xxx') keys; add corresponding entries in the
locale JSON files (en/*.json and fr/*.json) following the project's key naming
convention (e.g., taskCreation.title, taskCreation.description,
taskCreation.draftRestored, taskCreation.startFresh, taskCreation.hideFiles,
taskCreation.browseFiles, taskCreation.cancel, taskCreation.creating,
taskCreation.createTask, taskCreation.gitOptions.title,
taskCreation.gitOptions.baseBranch, taskCreation.gitOptions.help,
taskCreation.errors.missingDescription, taskCreation.errors.createFailed), and
update every occurrence referenced in the review (lines ~442, 461-472, 487, 494,
500-503, 576-578, 596-619) to call t(...) instead of inline strings.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/renderer/components/TaskCreationWizard.tsx
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/TaskCreationWizard.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/TaskCreationWizard.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/TaskCreationWizard.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:48.743Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/TaskCreationWizard.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/renderer/components/TaskCreationWizard.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/renderer/components/TaskCreationWizard.tsx
🧬 Code graph analysis (1)
apps/frontend/src/renderer/components/TaskCreationWizard.tsx (6)
apps/frontend/src/shared/types/task.ts (6)
TaskCategory(160-169)TaskPriority(156-156)TaskComplexity(154-154)TaskImpact(155-155)ModelType(159-159)ThinkingLevel(158-158)apps/frontend/src/renderer/stores/task-store.ts (2)
isDraftEmpty(577-588)saveDraft(517-533)apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx (1)
TaskModalLayout(39-136)apps/frontend/src/renderer/components/task-form/index.ts (2)
TaskModalLayout(11-11)TaskFormFields(12-12)apps/frontend/src/renderer/components/TaskFileExplorerDrawer.tsx (1)
TaskFileExplorerDrawer(37-116)apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx (1)
TaskFormFields(90-336)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (2)
apps/frontend/src/renderer/components/TaskCreationWizard.tsx (2)
1-49: LGTM!Imports are well-organized and the
PROJECT_DEFAULT_BRANCHsentinel value is clearly named.
318-362: LGTM!The
handleCreatefunction has correct validation, metadata assembly, and error handling. The sequential flow (validate → create → clear draft → close) is properly implemented.
| <span | ||
| key={i} | ||
| className="bg-info/20 text-info-foreground rounded px-0.5" | ||
| style={{ color: 'hsl(var(--info))' }} | ||
| > | ||
| {part} | ||
| </span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Minor: Redundant color styling.
The @mention span applies both a className with color intent (text-info-foreground) and an inline style overriding color. Pick one approach for consistency.
<span
key={i}
className="bg-info/20 text-info-foreground rounded px-0.5"
- style={{ color: 'hsl(var(--info))' }}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span | |
| key={i} | |
| className="bg-info/20 text-info-foreground rounded px-0.5" | |
| style={{ color: 'hsl(var(--info))' }} | |
| > | |
| {part} | |
| </span> | |
| <span | |
| key={i} | |
| className="bg-info/20 text-info-foreground rounded px-0.5" | |
| > | |
| {part} | |
| </span> |
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/TaskCreationWizard.tsx around lines
424 - 430, The span in TaskCreationWizard that renders @mentions currently sets
color via both className ("text-info-foreground") and an inline style (style={{
color: 'hsl(var(--info))' }); remove the redundant inline style and rely on the
CSS utility class for color consistency (or vice versa if you prefer inline
styles); update the span (the element with key={i} and className="bg-info/20
text-info-foreground rounded px-0.5") to use only one color approach.
| title="Create New Task" | ||
| description="Describe what you want to build. The AI will analyze your request and create a detailed specification." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded user-facing strings should use i18n translation keys.
Per coding guidelines, all user-facing text in frontend components must use i18n translation keys. This file introduces several new hardcoded strings:
- Modal title/description:
"Create New Task","Describe what you want to build..." - Draft UI:
"Draft restored","Start Fresh" - Buttons:
"Hide Files","Browse Files","Cancel","Creating...","Create Task" - Git options:
"Git Options (optional)","Base Branch (optional)", help text - Error messages:
"Please provide a description","Failed to create task..."
♻️ Example fix pattern
<TaskModalLayout
open={open}
onOpenChange={handleClose}
- title="Create New Task"
- description="Describe what you want to build. The AI will analyze your request..."
+ title={t('wizard.title')}
+ description={t('wizard.description')}
disabled={isCreating}
// ...
>Add corresponding keys to en/*.json and fr/*.json locale files.
As per coding guidelines, i18n translation keys are required for all user-facing text.
Also applies to: 461-472, 487-487, 494-494, 500-503, 576-578, 596-619
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/TaskCreationWizard.tsx around lines
442 - 443, Replace the hardcoded user-facing strings in the TaskCreationWizard
component with i18n translation keys: import and use the existing i18n hook
(useTranslation) in TaskCreationWizard and swap each literal (e.g., "Create New
Task", "Describe what you want to build...", "Draft restored", "Start Fresh",
"Hide Files", "Browse Files", "Cancel", "Creating...", "Create Task", "Git
Options (optional)", "Base Branch (optional)", the base-branch help text,
"Please provide a description", "Failed to create task...") for
t('taskCreation.xxx') keys; add corresponding entries in the locale JSON files
(en/*.json and fr/*.json) following the project's key naming convention (e.g.,
taskCreation.title, taskCreation.description, taskCreation.draftRestored,
taskCreation.startFresh, taskCreation.hideFiles, taskCreation.browseFiles,
taskCreation.cancel, taskCreation.creating, taskCreation.createTask,
taskCreation.gitOptions.title, taskCreation.gitOptions.baseBranch,
taskCreation.gitOptions.help, taskCreation.errors.missingDescription,
taskCreation.errors.createFailed), and update every occurrence referenced in the
review (lines ~442, 461-472, 487, 494, 500-503, 576-578, 596-619) to call t(...)
instead of inline strings.
| </p> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worktree isolation toggle removed from UI
Medium Severity
The useWorktree state variable still exists and is used in the task creation logic (if (!useWorktree) metadata.useWorktree = false), but the UI checkbox to toggle it has been removed from the Git Options section during the refactoring. Users can no longer disable worktree isolation even though the backend still supports it. The feature is now permanently locked to true with no way to change it.
Additional Locations (1)
AndyMik90
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 Auto Claude PR Review
🟠 Follow-up Review: Needs Revision
🟠 Needs revision - 6 unresolved finding(s) from previous review.
Resolution Status
- ✅ Resolved: 1 previous findings addressed
- ❌ Unresolved: 6 previous findings remain
- 🆕 New Issues: 5 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 1 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 6 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
🚨 Blocking Issues
- quality: [UNRESOLVED] Disconnected descriptionRef breaks @ mention autocomplete feature
- quality: [UNRESOLVED] New file violates i18n requirements - multiple hardcoded strings
- quality: [UNRESOLVED] Multiple hardcoded strings violate i18n requirements
- quality: [UNRESOLVED] Hardcoded screen reader text 'Close' violates i18n
- quality: [UNRESOLVED] Git options section has hardcoded strings violating i18n
- quality: Function signature mismatch - handleAutocompleteSelect ignores fullPath parameter
- quality: Incorrect isAutoProfile logic - applied to non-auto profiles
- quality: [FROM COMMENTS] CodeRabbit confirms i18n violations across multiple files
Verdict
CI Status: ⏳ 1 check pending - cannot approve until CI completes.
Previous Merge Conflicts: RESOLVED (merge commit a966649 merged origin/develop).
Findings Summary:
- 1 HIGH confirmed valid: descriptionRef disconnected from textarea breaks @ mention autocomplete (c3b68acc2e5e) - 3 agents confirmed
- 5 MEDIUM confirmed valid: 4 i18n violations + 1 new logic issue (isAutoProfile applied incorrectly)
- 3 LOW confirmed valid: 2 setTimeout cleanup issues + 1 eslint-disable
- 1 FALSE POSITIVE dismissed: hook file placement is actually consistent with codebase patterns
Blocking Issues:
- HIGH: The @ mention autocomplete feature is completely broken - descriptionRef.current is always null because the ref is never connected to the textarea (rendered in child component with separate ref)
Recommendations:
- Fix descriptionRef by either: (a) forward ref from TaskFormFields using React.forwardRef, (b) expose ref via useImperativeHandle, or (c) move autocomplete logic into TaskFormFields
- Address i18n violations by using t() for all hardcoded strings
- Wait for CI to complete
PR cannot be approved until the HIGH severity bug is fixed and CI passes.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer, comment-analyzer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (11 selected of 11 total)
🟠 [c3b68acc2e5e] [HIGH] [UNRESOLVED] Disconnected descriptionRef breaks @ mention autocomplete feature
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:111
The descriptionRef created at line 111 in TaskCreationWizard is never passed to TaskFormFields. TaskFormFields creates its own separate local ref at line 128 which is attached to the Textarea. The parent's descriptionRef.current will always be null, causing: (1) handleDescriptionChange to use cursorPos=0 instead of actual cursor position, (2) autocomplete positioning calculation to fail silently since textarea is null, (3) handleAutocompleteSelect to return early without setting focus/cursor position. This completely breaks the @ mention autocomplete feature for file references.
Resolution note: TaskCreationWizard.tsx line 113: const descriptionRef = useRef(null); // Never attached to anything
TaskFormFields.tsx line 128: const descriptionRef = useRef(null); // Separate ref
TaskFormFields.tsx line 158: <Textarea ref={descriptionRef} ... /> // Uses its own ref
TaskCreationWizard uses descriptionRef.current at lines 235-236, 273-274 for autocomplete positioning but it's always null.
Suggested fix:
Pass descriptionRef from TaskCreationWizard to TaskFormFields as a prop, or use React.forwardRef to expose the textarea ref. Add a new prop to TaskFormFieldsProps: `descriptionRef?: React.RefObject<HTMLTextAreaElement>` and use it in TaskFormFields instead of creating a local ref.
🟡 [3a70e35347c1] [MEDIUM] [UNRESOLVED] New file violates i18n requirements - multiple hardcoded strings
📁 apps/frontend/src/renderer/components/task-form/ClassificationFields.tsx:1
This new file introduces 10+ hardcoded user-facing strings that violate the project's mandatory i18n requirement stated in CLAUDE.md: 'CRITICAL: Always use i18n translation keys for all user-facing text in the frontend.' Affected strings: 'Category', 'Priority', 'Complexity', 'Impact', 'Select category/priority/complexity/impact' placeholders, and the help text 'These labels help organize and prioritize tasks...'
Resolution note: ClassificationFields.tsx lines 65-66: Category
Line 74: placeholder="Select category"
Lines 88, 111, 134: Priority, Complexity, Impact hardcoded
Line 156-158: Help text hardcoded
No useTranslation import or t() calls in file.
Suggested fix:
Add useTranslation hook and replace all hardcoded strings with translation keys from the tasks namespace. Add corresponding keys to apps/frontend/src/shared/i18n/locales/en/tasks.json and fr/tasks.json.
🟡 [e4941ab4e947] [MEDIUM] [UNRESOLVED] Multiple hardcoded strings violate i18n requirements
📁 apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx:152
TaskFormFields contains 8+ hardcoded user-facing strings that violate the project's mandatory i18n requirement. While the component imports useTranslation, most text is hardcoded: 'Description', 'Task Title', '(optional)', placeholders, 'Classification (optional)', 'Require human review before coding', and 'Image added successfully!'
Resolution note: TaskFormFields.tsx imports useTranslation but only uses it for images.pasteHint (line 180).
Hardcoded strings remain:
- Line 152: 'Description'
- Line 238: 'Task Title', '(optional)'
- Line 242: placeholder text
- Line 279: 'Classification (optional)'
- Lines 319-322: Review requirement text
Suggested fix:
Replace all hardcoded strings with translation keys. The component already has useTranslation imported - use t('form.description'), t('form.taskTitle'), t('form.classificationOptional'), t('form.requireReviewLabel'), t('images.addedSuccessfully'), etc.
🟡 [7f7d2f5000e9] [MEDIUM] [UNRESOLVED] Hardcoded screen reader text 'Close' violates i18n
📁 apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx:106
TaskModalLayout has a hardcoded 'Close' string for screen reader accessibility. While less visible to users, this still violates the project's i18n requirements and should use a translation key for proper localization.
Resolution note: TaskModalLayout.tsx line 106:
Close
No useTranslation import in file.
Suggested fix:
Add useTranslation hook and replace with: <span className="sr-only">{t('common:close')}</span>
🟡 [05e5ed3169f5] [MEDIUM] [UNRESOLVED] Git options section has hardcoded strings violating i18n
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:573
The Git options section unique to TaskCreationWizard contains hardcoded strings: 'Git Options (optional)', 'Base Branch (optional)', 'Use project default', and the help text explaining branch override functionality.
Resolution note: TaskCreationWizard.tsx:
Line 578: 'Git Options (optional)'
Line 597: 'Base Branch (optional)'
Line 605, 609: 'Use project default' text
Lines 618-619: Help text about branch override
Suggested fix:
Replace with translation keys: t('gitOptions.title'), t('gitOptions.baseBranch'), t('gitOptions.useProjectDefault'), t('gitOptions.helpText')
🔵 [e2eba72e919f] [LOW] [UNRESOLVED] setTimeout for success indicator not cleaned up on unmount
📁 apps/frontend/src/renderer/components/task-form/useImageUpload.ts:139
The setTimeout at line 139-140 sets pasteSuccess to false after 2 seconds, but this timeout is never cleared if the component unmounts. While React 18+ doesn't warn about this, it's a code quality issue that could be cleaned up.
Resolution note: useImageUpload.ts lines 138-140:
setPasteSuccess(true);
setTimeout(() => setPasteSuccess(false), 2000);
No timeout ID storage or cleanup mechanism.
Suggested fix:
Store timeout ID and clear on cleanup, or use a useEffect with pasteSuccess dependency to manage the timer lifecycle
🟡 [NEW-001] [MEDIUM] Function signature mismatch - handleAutocompleteSelect ignores fullPath parameter
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:271
The handleAutocompleteSelect callback accepts only (filename: string) but FileAutocomplete.onSelect expects (filename: string, fullPath: string). The fullPath parameter is silently ignored, which could cause issues when files with the same name exist in different directories.
Suggested fix:
Update signature: const handleAutocompleteSelect = useCallback((filename: string, fullPath: string) => { ... }
🔵 [NEW-004] [LOW] setTimeout in handleAutocompleteSelect not cleaned up on unmount
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:283
The setTimeout used to restore cursor position after autocomplete selection is not tracked or cleaned up if the component unmounts.
Suggested fix:
Use queueMicrotask() instead of setTimeout(fn, 0) as it doesn't need cleanup, or store timeout ID in ref and clear on unmount.
🟡 [NEW-005] [MEDIUM] Incorrect isAutoProfile logic - applied to non-auto profiles
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:337
The code sets metadata.isAutoProfile = true whenever phaseModels && phaseThinking exist, without checking if profileId === 'auto'. This incorrectly marks tasks from other profiles (balanced, quick, custom) as auto profile tasks. Same issue in TaskEditDialog.tsx lines 199-205.
Suggested fix:
Add profileId check: if (profileId === 'auto' && phaseModels && phaseThinking) { metadata.isAutoProfile = true; ... }
🔵 [CMT-003] [LOW] eslint-disable suppresses valid exhaustive-deps warning
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:162
The useEffect at lines 157-163 disables react-hooks/exhaustive-deps rule to suppress a valid warning. fetchBranches and fetchProjectDefaultBranch should be wrapped in useCallback or moved inside the useEffect.
Suggested fix:
Wrap fetchBranches and fetchProjectDefaultBranch in useCallback with proper dependencies, or move function definitions inside useEffect.
🟡 [CMT-002] [MEDIUM] [FROM COMMENTS] CodeRabbit confirms i18n violations across multiple files
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:442
CodeRabbit flagged multiple hardcoded strings violating i18n: modal title/description, draft UI, file explorer buttons, action buttons, and git options. This confirms findings 3a70e35347c1, e4941ab4e947, 7f7d2f5000e9, and 05e5ed3169f5.
This review was generated by Auto Claude.
- Fix HIGH: Pass descriptionRef from TaskCreationWizard to TaskFormFields to fix broken @ mention autocomplete positioning - Fix MEDIUM: Add i18n translations for all hardcoded strings in: - ClassificationFields.tsx - TaskFormFields.tsx - TaskModalLayout.tsx - TaskCreationWizard.tsx (modal, draft, buttons, git options) - TaskEditDialog.tsx - Fix MEDIUM: Correct isAutoProfile logic to only set true when profileId === 'auto' (not for all profiles with phase configs) - Fix MEDIUM: Update handleAutocompleteSelect signature to accept optional fullPath parameter - Fix LOW: Add proper setTimeout cleanup in useImageUpload.ts - Fix LOW: Use queueMicrotask instead of setTimeout in handleAutocompleteSelect for cursor position restoration - Fix LOW: Move fetch functions inside useEffect to fix exhaustive-deps warning - Add English and French translations for all new i18n keys 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/frontend/src/renderer/components/TaskEditDialog.tsx (2)
166-170: Hardcoded validation error message.This error string should use an i18n translation key per coding guidelines.
Suggested fix
if (!description.trim()) { - setError('Description is required'); + setError(t('common:validation.descriptionRequired')); return; }
220-222: Hardcoded failure error message.Same i18n concern - use a translation key for user-facing error text.
Suggested fix
} else { - setError('Failed to update task. Please try again.'); + setError(t('common:errors.taskUpdateFailed')); }apps/frontend/src/renderer/components/TaskCreationWizard.tsx (2)
319-322: Hardcoded validation error message.Per coding guidelines, use i18n translation keys for user-facing text.
Suggested fix
if (!description.trim()) { - setError('Please provide a description'); + setError(t('common:validation.descriptionRequired')); return; }
354-356: Hardcoded failure error message.Same i18n concern for the task creation failure message.
🤖 Fix all issues with AI agents
In @apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx:
- Around line 217-219: The aria-label translation key in the TaskFormFields
component is missing the tasks namespace; update the call that sets aria-label
(inside the remove image button/handler in TaskFormFields) to use
'tasks:images.removeImageAriaLabel' instead of 'images.removeImageAriaLabel' so
the i18n lookup is explicit and consistent with the rest of the tasks namespace.
- Around line 184-186: The translation key used in the TaskFormFields component
is missing the explicit namespace prefix; update the call to the i18n translator
(the t(...) invocation that currently uses 'images.pasteHint') to use the
namespaced key 'tasks:images.pasteHint' so it always resolves to the correct
namespace regardless of namespace order; locate the t call inside the <p
id={`${prefix}description-help`} ...> block and replace the key string only,
leaving the platform check for the shortcut unchanged.
In @apps/frontend/src/renderer/components/task-form/useImageUpload.ts:
- Around line 109-113: The error message for invalid image types in
useImageUpload uses a hardcoded string; replace it with the app's i18n call
(e.g., t or formatMessage) so the message is localized and still includes
ALLOWED_IMAGE_TYPES_DISPLAY; update the call in the block where
isValidImageMimeType(file.type) is checked to call the translation helper (or
use a provided i18n prop/context) and pass ALLOWED_IMAGE_TYPES_DISPLAY as an
interpolation parameter to the localized string instead of the hardcoded English
text.
- Around line 141-143: The catch in useImageUpload currently swallows the error
and emits hardcoded messages; update the catch to accept the error (e.g., catch
(err)), call onError?. with i18n keys instead of literal strings (use the
appropriate translation keys for pasted vs dropped images), and log the error
for debugging (use the existing logger or console.error) including context
(e.g., options.isFromPaste and file info) so the actual exception is preserved
and traceable.
In @apps/frontend/src/renderer/components/TaskCreationWizard.tsx:
- Around line 156-194: Extract the two async helpers fetchBranches and
fetchProjectDefaultBranch out of the useEffect and wrap each in useCallback so
they are stable and reusable: create const fetchBranches = useCallback(async ()
=> { ... }, [projectPath, setIsLoadingBranches, setBranches]) that calls
window.electronAPI.getGitBranches and sets state, and const
fetchProjectDefaultBranch = useCallback(async () => { ... }, [projectId,
projectPath, setProjectDefaultBranch]) that calls
window.electronAPI.getProjectEnv and, on fallback,
window.electronAPI.detectMainBranch; then have the useEffect simply call
fetchBranches() and fetchProjectDefaultBranch() when open && projectPath, and
ensure the useEffect dependency array includes open, projectPath, projectId and
the two callbacks.
- Line 515: The descriptionPlaceholder in TaskCreationWizard is hardcoded;
replace it with an i18n translation key (e.g.,
"taskCreation.descriptionPlaceholder") by adding that key to your localization
files and using the translation hook/utility already used in the component
(e.g., useTranslation or t) to pass the translated string into the
descriptionPlaceholder prop; update TaskCreationWizard to import/use the
translator and swap the literal string for
t('taskCreation.descriptionPlaceholder') so the instructional text (including
the "@" mention hint) is localized.
In @apps/frontend/src/shared/i18n/locales/fr/tasks.json:
- Around line 129-130: The translation files contain duplicate keys
"creation.title" and "wizard.createTitle" with identical strings; decide whether
to consolidate into a single key or document distinct usage: search for
references to creation.title and wizard.createTitle in the codebase, remove one
key and update callers to the chosen canonical key (or keep both but add a
comment in the fr and en JSON explaining their distinct contexts), and ensure
you update any tests or components that reference the removed key such as UI
screens or wizard components to use the retained key.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
apps/frontend/src/renderer/components/TaskCreationWizard.tsxapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/task-form/TaskModalLayout.tsxapps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/shared/i18n/locales/en/tasks.jsonapps/frontend/src/shared/i18n/locales/fr/tasks.json
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/TaskCreationWizard.tsxapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/TaskCreationWizard.tsxapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/TaskCreationWizard.tsxapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/shared/i18n/locales/**/*.json: Store translation strings in namespace-organized JSON files atapps/frontend/src/shared/i18n/locales/{lang}/*.jsonfor each supported language
When implementing new frontend features, add translation keys to all language files (minimum: en/.json and fr/.json)
Files:
apps/frontend/src/shared/i18n/locales/fr/tasks.jsonapps/frontend/src/shared/i18n/locales/en/tasks.json
🧠 Learnings (5)
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/TaskCreationWizard.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/TaskCreationWizard.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/shared/i18n/locales/fr/tasks.jsonapps/frontend/src/shared/i18n/locales/en/tasks.json
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/task-form/TaskFormFields.tsxapps/frontend/src/renderer/components/TaskCreationWizard.tsxapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsxapps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx
📚 Learning: 2026-01-04T12:37:58.185Z
Learnt from: StillKnotKnown
Repo: AndyMik90/Auto-Claude PR: 648
File: apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx:262-265
Timestamp: 2026-01-04T12:37:58.185Z
Learning: In apps/frontend/src/renderer/components/settings/ModelSearchableSelect.tsx and similar dropdown components: the dropdowns use max-h-60 (or similar max-height constraints) with internal overflow-y-auto scrolling, which prevents the dropdown from extending beyond available viewport space. This fixed-height + internal-scroll pattern is intentional and consistent across all dropdowns in the application, so dynamic viewport positioning or clipping prevention logic is not needed.
Applied to files:
apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/renderer/components/TaskCreationWizard.tsxapps/frontend/src/renderer/components/TaskEditDialog.tsxapps/frontend/src/renderer/components/task-form/ClassificationFields.tsx
🧬 Code graph analysis (3)
apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx (6)
apps/frontend/src/shared/types/task.ts (6)
ModelType(159-159)TaskCategory(160-169)TaskPriority(156-156)TaskComplexity(154-154)TaskImpact(155-155)ImageAttachment(113-121)apps/frontend/src/renderer/components/task-form/index.ts (3)
TaskFormFields(12-12)useImageUpload(14-14)ClassificationFields(13-13)apps/frontend/src/renderer/components/task-form/useImageUpload.ts (1)
useImageUpload(53-256).design-system/src/components/Input.tsx (1)
Input(4-24)apps/frontend/src/renderer/components/AgentProfileSelector.tsx (1)
AgentProfileSelector(72-380)apps/frontend/src/renderer/components/task-form/ClassificationFields.tsx (1)
ClassificationFields(47-163)
apps/frontend/src/renderer/components/TaskEditDialog.tsx (3)
apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx (1)
TaskModalLayout(40-139)apps/frontend/src/renderer/components/task-form/index.ts (2)
TaskModalLayout(11-11)TaskFormFields(12-12)apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx (1)
TaskFormFields(92-341)
apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx (3)
apps/frontend/src/renderer/components/task-form/index.ts (1)
TaskModalLayout(11-11).design-system/src/lib/icons.ts (1)
X(37-37)apps/frontend/src/renderer/components/ui/scroll-area.tsx (1)
ScrollArea(47-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (9)
apps/frontend/src/renderer/components/task-form/ClassificationFields.tsx (1)
1-163: Well-structured shared component with proper i18n and accessibility.The component correctly uses:
- Translation keys via
useTranslation('tasks')for all labels and help text- Proper
htmlFor/idassociations with configurable prefix for accessibility- TypeScript types for all classification fields
Note: The
TASK_*_LABELSconstants (lines 79-83, 102-106, etc.) use hardcoded display strings rather than i18n keys. Based on learnings, this is a pre-existing pattern that can be deferred to a future i18n cleanup pass.apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx (1)
40-138: LGTM! Clean modal layout abstraction.The component provides:
- Consistent full-height modal sizing (95vw, max-w-5xl)
- Proper Radix Dialog primitives with accessibility semantics (Title, Description)
- Disabled state handling that prevents close during operations
- Optional sidebar support for file explorer
apps/frontend/src/renderer/components/TaskEditDialog.tsx (1)
229-293: Clean refactor to shared components.The composition with
TaskModalLayoutandTaskFormFieldsis well-structured, properly passing all required props including theidPrefix="edit"for accessibility. The footer actions use proper i18n keys.apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx (1)
92-340: Well-designed shared form component.The component properly:
- Centralizes form field logic for reuse across TaskCreationWizard and TaskEditDialog
- Integrates the
useImageUploadhook for unified image handling- Provides accessibility attributes (
aria-required,aria-describedby,aria-expanded,aria-controls)- Accepts an optional
descriptionReffor external autocomplete positioning- Supports optional children slot for overlays
apps/frontend/src/renderer/components/TaskCreationWizard.tsx (1)
438-633: Successful refactor to modular composition.The component now properly uses:
TaskModalLayoutfor consistent modal chromeTaskFormFieldsfor shared form fields with the autocomplete popup as children- Dedicated Git options section unique to task creation
- Draft persistence and restore flow
The file explorer sidebar integration via
TaskModalLayout's sidebar prop is clean.apps/frontend/src/shared/i18n/locales/en/tasks.json (1)
129-172: French translations are already present for all new keys.The verification confirms that
apps/frontend/src/shared/i18n/locales/fr/tasks.jsoncontains complete French translations for all new keys added to the English file, including thewizard,edit, andformsections with their nested properties.Likely an incorrect or invalid review comment.
apps/frontend/src/shared/i18n/locales/fr/tasks.json (3)
138-142: LGTM! Git options translations are well-structured.The French translations for the Git options are accurate and maintain proper template variable syntax ({{branch}}). The helpText clearly explains the purpose of the base branch override functionality.
147-150: LGTM! Edit section translations are clear and accurate.The edit section translations appropriately describe the task editing functionality. The description effectively communicates that changes will be saved to specification files.
152-171: LGTM! French translations include all required keys with proper English parity.The form section translations are comprehensive and grammatically correct. The nested classification structure effectively supports the 2x2 grid layout. All help text and descriptions clearly communicate the functionality. The English translation file contains all matching keys at both the form level and form.classification level, satisfying translation parity requirements.
| <p id={`${prefix}description-help`} className="text-xs text-muted-foreground"> | ||
| {t('images.pasteHint', { shortcut: navigator.platform.includes('Mac') ? '⌘V' : 'Ctrl+V' })} | ||
| </p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing namespace prefix in translation key.
The translation key 'images.pasteHint' should include the namespace prefix 'tasks:' to match the other keys in this component. While it may work if tasks is the first namespace in the array, explicit namespacing is more robust.
Suggested fix
<p id={`${prefix}description-help`} className="text-xs text-muted-foreground">
- {t('images.pasteHint', { shortcut: navigator.platform.includes('Mac') ? '⌘V' : 'Ctrl+V' })}
+ {t('tasks:images.pasteHint', { shortcut: navigator.platform.includes('Mac') ? '⌘V' : 'Ctrl+V' })}
</p>🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx around
lines 184 - 186, The translation key used in the TaskFormFields component is
missing the explicit namespace prefix; update the call to the i18n translator
(the t(...) invocation that currently uses 'images.pasteHint') to use the
namespaced key 'tasks:images.pasteHint' so it always resolves to the correct
namespace regardless of namespace order; locate the t call inside the <p
id={`${prefix}description-help`} ...> block and replace the key string only,
leaving the platform check for the shortcut unchanged.
| }} | ||
| aria-label={t('images.removeImageAriaLabel', { filename: image.filename })} | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing namespace prefix in translation key.
Same issue - 'images.removeImageAriaLabel' should be 'tasks:images.removeImageAriaLabel' for consistency and explicit namespace resolution.
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx around
lines 217 - 219, The aria-label translation key in the TaskFormFields component
is missing the tasks namespace; update the call that sets aria-label (inside the
remove image button/handler in TaskFormFields) to use
'tasks:images.removeImageAriaLabel' instead of 'images.removeImageAriaLabel' so
the i18n lookup is explicit and consistent with the rest of the tasks namespace.
apps/frontend/src/renderer/components/task-form/useImageUpload.ts
Outdated
Show resolved
Hide resolved
| "createTitle": "Créer une nouvelle tâche", | ||
| "createDescription": "Décrivez ce que vous voulez construire. L'IA analysera votre demande et créera une spécification détaillée.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check English translations for both keys
echo "=== English translations ==="
if [ -f apps/frontend/src/shared/i18n/locales/en/tasks.json ]; then
jq '.creation.title // "NOT FOUND", .wizard.createTitle // "NOT FOUND"' apps/frontend/src/shared/i18n/locales/en/tasks.json
else
echo "File not found"
fi
# Find component usage of creation.title
echo -e "\n=== Usage of creation.title ==="
rg -n "creation\.title" --type=ts --type=tsx -g '!**/locales/**' 2>/dev/null || echo "No matches found"
# Find component usage of wizard.createTitle
echo -e "\n=== Usage of wizard.createTitle ==="
rg -n "wizard\.createTitle" --type=ts --type=tsx -g '!**/locales/**' 2>/dev/null || echo "No matches found"
# Also check French file for both keys
echo -e "\n=== French translations ==="
if [ -f apps/frontend/src/shared/i18n/locales/fr/tasks.json ]; then
jq '.creation.title // "NOT FOUND", .wizard.createTitle // "NOT FOUND"' apps/frontend/src/shared/i18n/locales/fr/tasks.json
else
echo "File not found"
fiRepository: AndyMik90/Auto-Claude
Length of output: 316
Consolidate duplicate translation keys or clarify their distinct purposes.
Both creation.title and wizard.createTitle have identical translations in both English ("Create New Task") and French ("Créer une nouvelle tâche") files, yet neither key is currently used in the codebase. To avoid maintenance burden, either consolidate these keys into a single one or document why they serve distinct contexts that justify separate keys.
🤖 Prompt for AI Agents
In @apps/frontend/src/shared/i18n/locales/fr/tasks.json around lines 129 - 130,
The translation files contain duplicate keys "creation.title" and
"wizard.createTitle" with identical strings; decide whether to consolidate into
a single key or document distinct usage: search for references to creation.title
and wizard.createTitle in the codebase, remove one key and update callers to the
chosen canonical key (or keep both but add a comment in the fr and en JSON
explaining their distinct contexts), and ensure you update any tests or
components that reference the removed key such as UI screens or wizard
components to use the retained key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/TaskCreationWizard.tsx (1)
121-155: Draft-loading effect can overwrite in-progress edits due to broad dependencies.
This effect re-runs on changes tosettings.*/selectedProfile.*(Line 154), and will re-apply draft/default values while the modal is open, potentially clobbering user input.Proposed fix (run only on open transition)
export function TaskCreationWizard({ @@ }: TaskCreationWizardProps) { @@ + const prevOpenRef = useRef<boolean>(false); @@ useEffect(() => { - if (open && projectId) { + const justOpened = open && !prevOpenRef.current; + prevOpenRef.current = open; + if (justOpened && projectId) { const draft = loadDraft(projectId); if (draft && !isDraftEmpty(draft)) { @@ - setImages(draft.images); + setImages(draft.images ?? []); setReferencedFiles(draft.referencedFiles ?? []); @@ } else { @@ } } - }, [open, projectId, settings.selectedAgentProfile, settings.customPhaseModels, settings.customPhaseThinking, selectedProfile.model, selectedProfile.thinkingLevel, selectedProfile.phaseModels, selectedProfile.phaseThinking]); + }, [open, projectId]);
🤖 Fix all issues with AI agents
In @apps/frontend/src/renderer/components/task-form/useImageUpload.ts:
- Around line 82-87: The merged `errors` memo currently depends on the object
identity of `errorMessages`, so inline callers (e.g., TaskFormFields) invalidate
`errors` each render and thus still recreate `processImageItems`; fix the hook
by stabilizing the dependency list: replace the single-object dep
`[errorMessages]` in the useMemo that produces `errors` (and the similar memo at
182-183) with a deterministic primitive-level dependency (for example use the
specific string fields or Object.values(errorMessages) as the dependency array)
so the merged `errors` only changes when actual message text changes;
alternatively, change the hook API to accept individual string props instead of
an object or require callers to wrap `errorMessages` in useMemo — choose one
approach and apply consistently to `useMemo` that builds `errors` and any
memo/callbacks that depend on it (e.g., `processImageItems`) so they stop being
invalidated by object identity churn.
- Around line 140-170: When deriving the file extension from file.type in the
image processing block (around blobToBase64/createThumbnail and before
resolveFilename), sanitize the MIME subtype so SVGs and other +suffix types
produce correct extensions: get the subtype via file.type.split('/')[1], then if
it contains a '+' take the substring before the '+' and normalize to lowercase;
if the result is empty fall back to 'png'. Use that sanitized extension when
building baseFilename (used before resolveFilename and when pushing newImages
entries).
- Around line 63-69: DEFAULT_ERROR_MESSAGES currently contains hardcoded English
strings; update useImageUpload to build its fallback errorMessages via the i18n
translation function (e.g., useTranslation or i18n.t) instead of literals.
Replace the static DEFAULT_ERROR_MESSAGES with a runtime default assembled
inside the useImageUpload hook using the existing translation keys from
tasks.json (e.g., 'tasks.maxImagesReached', 'tasks.invalidImageType',
'tasks.processPasteFailed', 'tasks.processDropFailed'), and then merge that
translated default with the incoming errorMessages prop so callers still
override specific messages.
In @apps/frontend/src/renderer/components/TaskCreationWizard.tsx:
- Around line 156-200: The useEffect fetchBranches flow can leave
isLoadingBranches true if the dialog closes before fetch finishes because the
finally block is skipped by the isMounted guard; update the effect so the
cleanup always resets loading: keep setting isLoadingBranches(true) at the start
of fetchBranches, remove the guarded setIsLoadingBranches(false) from the
finally block, and instead call setIsLoadingBranches(false) unconditionally
inside the useEffect cleanup; retain the isMounted/isActive checks only for
mutating state like setBranches and setProjectDefaultBranch in fetchBranches and
fetchProjectDefaultBranch to avoid setting state on unmounted components.
- Around line 239-272: handleDescriptionChange computes popup coords in
textarea-local space but applies them as absolute/viewport coords; update the
positioning to convert to viewport coordinates by adding the textarea bounding
rect offsets and subtracting textarea scroll offsets before calling
setAutocomplete. Specifically, in the handleDescriptionChange closure (uses
descriptionRef, detectAtMention and setAutocomplete) compute top = rect.top +
paddingTop + currentLineIndex * lineHeight + extraOffset - textarea.scrollTop
and left = rect.left + paddingLeft + Math.min(currentLineLength * charWidth,
rect.width - popupWidth) - textarea.scrollLeft, then pass those values into
setAutocomplete.position (clamped with Math.max(0, ...)). Alternatively, if you
prefer content-relative placement, render FileAutocomplete inside the textarea's
positioned ancestor and keep current content-relative math; ensure you handle
rect.top/left and textarea.scrollTop/scrollLeft in whichever approach you
choose.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apps/frontend/src/renderer/components/TaskCreationWizard.tsxapps/frontend/src/renderer/components/task-form/useImageUpload.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/TaskCreationWizard.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/TaskCreationWizard.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/TaskCreationWizard.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:48.743Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/TaskCreationWizard.tsx
📚 Learning: 2026-01-04T23:59:48.743Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:48.743Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
Applied to files:
apps/frontend/src/renderer/components/task-form/useImageUpload.ts
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/TaskCreationWizard.tsx
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/renderer/components/task-form/useImageUpload.tsapps/frontend/src/renderer/components/TaskCreationWizard.tsx
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/TaskCreationWizard.tsx
🧬 Code graph analysis (2)
apps/frontend/src/renderer/components/task-form/useImageUpload.ts (2)
apps/frontend/src/shared/types/task.ts (1)
ImageAttachment(113-121)apps/frontend/src/shared/constants/task.ts (2)
MAX_IMAGES_PER_TASK(192-192)ALLOWED_IMAGE_TYPES_DISPLAY(211-211)
apps/frontend/src/renderer/components/TaskCreationWizard.tsx (3)
apps/frontend/src/shared/types/task.ts (9)
TaskCategory(160-169)TaskPriority(156-156)TaskComplexity(154-154)TaskImpact(155-155)ModelType(159-159)ThinkingLevel(158-158)ImageAttachment(113-121)ReferencedFile(124-130)TaskMetadata(171-237)apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx (1)
TaskModalLayout(40-139)apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx (1)
TaskFormFields(92-347)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (5)
apps/frontend/src/renderer/components/task-form/useImageUpload.ts (2)
216-265: Verify “non-image drop uses browser default” with currentonDragOverbehavior.
handleDragOveralways callse.preventDefault()(Line 217). Depending on browser/Electron behavior, that can still disrupt default drop insertion even ifhandleDropreturns early without preventing default (Line 257).Action: manually test dropping:
- a non-image file from OS file explorer into the textarea
- a text selection / file reference token into the textarea
270-287: Callbacks/return shape look clean and consistent.
removeImageis minimal, and the public API (canAddMore,remainingSlots, handlers) is cohesive.apps/frontend/src/renderer/components/TaskCreationWizard.tsx (3)
276-295:queueMicrotaskfor cursor restore is a solid simplification.
AvoidssetTimeoutcleanup and keeps selection updates responsive.
324-354: Metadata assembly changes look coherent (auto profile + baseBranch sentinel + worktree flag).
metadata.isAutoProfile = profileId === 'auto'andbaseBranch !== PROJECT_DEFAULT_BRANCHare clear and keep the “project default” behavior explicit.
444-639: Modal/layout wiring looks consistent; verify close semantics withTaskModalLayout.
TaskModalLayoutwraps RadixonOpenChangeto always close; passinghandleClose(which ignores the boolean) is probably fine, but it’s worth confirming that:
- closing via overlay/ESC triggers the same draft-save path
- reopening doesn’t get immediately canceled by any
onOpenChangeinvocation patterns
| // Default error messages (English fallbacks) | ||
| const DEFAULT_ERROR_MESSAGES: Required<ImageUploadErrorMessages> = { | ||
| maxImagesReached: `Maximum of ${MAX_IMAGES_PER_TASK} images allowed`, | ||
| invalidImageType: `Invalid image type. Allowed: ${ALLOWED_IMAGE_TYPES_DISPLAY}`, | ||
| processPasteFailed: 'Failed to process pasted image', | ||
| processDropFailed: 'Failed to process dropped image' | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and verify it exists
find . -name "useImageUpload.ts" -type fRepository: AndyMik90/Auto-Claude
Length of output: 206
🏁 Script executed:
# Check the file content at the specified lines
cat -n apps/frontend/src/renderer/components/task-form/useImageUpload.ts | sed -n '1,100p'Repository: AndyMik90/Auto-Claude
Length of output: 4165
🏁 Script executed:
# Check git status to determine if this is new code or pre-existing
cd apps/frontend/src/renderer/components/task-form && git log --oneline -5 useImageUpload.ts 2>/dev/null || echo "Not in git history or file doesn't exist"Repository: AndyMik90/Auto-Claude
Length of output: 144
🏁 Script executed:
# Search for i18n usage in the file to understand current localization approach
grep -n "useTranslation\|i18n\|t(" apps/frontend/src/renderer/components/task-form/useImageUpload.tsRepository: AndyMik90/Auto-Claude
Length of output: 602
🏁 Script executed:
# Check the codebase for i18n patterns in similar error handling
grep -r "DEFAULT_ERROR_MESSAGES\|error.*messages" apps/frontend/src --include="*.ts" --include="*.tsx" | head -20Repository: AndyMik90/Auto-Claude
Length of output: 1957
🏁 Script executed:
# Check how TaskFormFields calls useImageUpload to understand the current design
cat -n apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx | grep -A 15 -B 5 "useImageUpload"Repository: AndyMik90/Auto-Claude
Length of output: 1591
🏁 Script executed:
# Search for where errorMessages is actually being passed with translations
grep -r "errorMessages" apps/frontend/src/renderer/components/task-form --include="*.ts" --include="*.tsx" -B 3 -A 3Repository: AndyMik90/Auto-Claude
Length of output: 2999
🏁 Script executed:
# Check if translation files have i18n keys for these error messages
find apps/frontend/src/shared/i18n/locales -name "*.json" -type f | head -10Repository: AndyMik90/Auto-Claude
Length of output: 611
🏁 Script executed:
# Look for existing task form error translations
grep -r "maxImagesReached\|processPasteFailed\|processDropFailed\|invalidImageType" apps/frontend/src/shared/i18n/locales --include="*.json"Repository: AndyMik90/Auto-Claude
Length of output: 1021
Remove hardcoded English defaults; make errors i18n-native.
DEFAULT_ERROR_MESSAGES contains hardcoded user-facing strings in apps/frontend/src/renderer/components/task-form/useImageUpload.ts and violates the coding guideline requiring i18n translation keys for all frontend text. Although the hook accepts errorMessages prop for translations and TaskFormFields.tsx correctly uses it, the DEFAULT_ERROR_MESSAGES fallback (lines 63–69) remains hardcoded English. Translation keys already exist in both en/tasks.json and fr/tasks.json, so integrate i18n directly into the hook to ensure compliance everywhere it's used.
Proposed fix (use i18n inside the hook)
import { useState, useCallback, useRef, useEffect, useMemo, type ClipboardEvent, type DragEvent } from 'react';
+import { useTranslation } from 'react-i18next';
import {
generateImageId,
blobToBase64,
createThumbnail,
isValidImageMimeType,
resolveFilename
} from '../ImageUpload';
@@
-// Default error messages (English fallbacks)
-const DEFAULT_ERROR_MESSAGES: Required<ImageUploadErrorMessages> = {
- maxImagesReached: `Maximum of ${MAX_IMAGES_PER_TASK} images allowed`,
- invalidImageType: `Invalid image type. Allowed: ${ALLOWED_IMAGE_TYPES_DISPLAY}`,
- processPasteFailed: 'Failed to process pasted image',
- processDropFailed: 'Failed to process dropped image'
-};
-
export function useImageUpload({
@@
}: UseImageUploadOptions): UseImageUploadReturn {
+ const { t } = useTranslation(['tasks']);
const [isDragOver, setIsDragOver] = useState(false);
@@
- const errors = useMemo<Required<ImageUploadErrorMessages>>(() => ({
- ...DEFAULT_ERROR_MESSAGES,
- ...errorMessages
- }), [errorMessages]);
+ const defaultErrors = useMemo<Required<ImageUploadErrorMessages>>(
+ () => ({
+ maxImagesReached: t('tasks:form.errors.maxImagesReached'),
+ invalidImageType: t('tasks:form.errors.invalidImageType'),
+ processPasteFailed: t('tasks:form.errors.processPasteFailed'),
+ processDropFailed: t('tasks:form.errors.processDropFailed')
+ }),
+ [t]
+ );
+
+ const errors = useMemo<Required<ImageUploadErrorMessages>>(
+ () => ({ ...defaultErrors, ...errorMessages }),
+ [defaultErrors, errorMessages]
+ );🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/task-form/useImageUpload.ts around
lines 63 - 69, DEFAULT_ERROR_MESSAGES currently contains hardcoded English
strings; update useImageUpload to build its fallback errorMessages via the i18n
translation function (e.g., useTranslation or i18n.t) instead of literals.
Replace the static DEFAULT_ERROR_MESSAGES with a runtime default assembled
inside the useImageUpload hook using the existing translation keys from
tasks.json (e.g., 'tasks.maxImagesReached', 'tasks.invalidImageType',
'tasks.processPasteFailed', 'tasks.processDropFailed'), and then merge that
translated default with the incoming errorMessages prop so callers still
override specific messages.
| // Merge custom error messages with defaults (memoized to prevent useCallback invalidation) | ||
| const errors = useMemo<Required<ImageUploadErrorMessages>>(() => ({ | ||
| ...DEFAULT_ERROR_MESSAGES, | ||
| ...errorMessages | ||
| }), [errorMessages]); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
useMemo here won’t stabilize callbacks unless the caller memoizes errorMessages.
If callers pass {...} inline (as TaskFormFields does), errorMessages identity changes every render, so errors changes and processImageItems is still invalidated.
Action: either (a) memoize errorMessages at call sites, or (b) accept primitives (strings) instead of an object to avoid identity churn.
Also applies to: 182-183
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/task-form/useImageUpload.ts around
lines 82 - 87, The merged `errors` memo currently depends on the object identity
of `errorMessages`, so inline callers (e.g., TaskFormFields) invalidate `errors`
each render and thus still recreate `processImageItems`; fix the hook by
stabilizing the dependency list: replace the single-object dep `[errorMessages]`
in the useMemo that produces `errors` (and the similar memo at 182-183) with a
deterministic primitive-level dependency (for example use the specific string
fields or Object.values(errorMessages) as the dependency array) so the merged
`errors` only changes when actual message text changes; alternatively, change
the hook API to accept individual string props instead of an object or require
callers to wrap `errorMessages` in useMemo — choose one approach and apply
consistently to `useMemo` that builds `errors` and any memo/callbacks that
depend on it (e.g., `processImageItems`) so they stop being invalidated by
object identity churn.
| try { | ||
| const dataUrl = await blobToBase64(file); | ||
| const thumbnail = await createThumbnail(dataUrl); | ||
|
|
||
| // Generate filename based on source | ||
| let baseFilename: string; | ||
| if (options.isFromPaste || !file.name || file.name === 'image.png') { | ||
| const extension = file.type.split('/')[1] || 'png'; | ||
| baseFilename = `screenshot-${Date.now()}.${extension}`; | ||
| } else { | ||
| baseFilename = file.name; | ||
| } | ||
|
|
||
| const resolvedFilename = resolveFilename(baseFilename, [ | ||
| ...existingFilenames, | ||
| ...newImages.map((img) => img.filename) | ||
| ]); | ||
|
|
||
| newImages.push({ | ||
| id: generateImageId(), | ||
| filename: resolvedFilename, | ||
| mimeType: file.type, | ||
| size: file.size, | ||
| data: dataUrl.split(',')[1], // Store base64 without data URL prefix | ||
| thumbnail | ||
| }); | ||
| } catch (error) { | ||
| console.error('Image processing error:', error); | ||
| onError?.(options.isFromPaste ? errors.processPasteFailed : errors.processDropFailed); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full context of the useImageUpload.ts file
wc -l apps/frontend/src/renderer/components/task-form/useImageUpload.tsRepository: AndyMik90/Auto-Claude
Length of output: 134
🏁 Script executed:
# Read the relevant section and surrounding context
head -180 apps/frontend/src/renderer/components/task-form/useImageUpload.ts | tail -100Repository: AndyMik90/Auto-Claude
Length of output: 3129
🏁 Script executed:
# Find the blobToBase64 function implementation
rg -n "blobToBase64" apps/frontend/src --type ts --type tsx -A 10Repository: AndyMik90/Auto-Claude
Length of output: 92
🏁 Script executed:
# Search for blobToBase64 with correct file type flags
rg -n "blobToBase64" apps/frontend/src -A 10Repository: AndyMik90/Auto-Claude
Length of output: 5813
🏁 Script executed:
# Let's see the imports at the top of the file
head -50 apps/frontend/src/renderer/components/task-form/useImageUpload.tsRepository: AndyMik90/Auto-Claude
Length of output: 1762
🏁 Script executed:
# Search for isValidImageMimeType implementation
rg -n "isValidImageMimeType" apps/frontend/src -A 5Repository: AndyMik90/Auto-Claude
Length of output: 3051
🏁 Script executed:
# Find ALLOWED_IMAGE_TYPES constant
rg -n "ALLOWED_IMAGE_TYPES" apps/frontend/src --type ts -B 2 -A 5Repository: AndyMik90/Auto-Claude
Length of output: 8830
🏁 Script executed:
# Look at the ImageUpload.tsx file to see the complete picture
cat apps/frontend/src/renderer/components/ImageUpload.tsxRepository: AndyMik90/Auto-Claude
Length of output: 11962
Fix MIME-type extension handling for SVG images.
The code on line 147 splits file.type to extract the extension: file.type.split('/')[1] will produce 'svg+xml' for SVG files instead of the correct 'svg'. This creates invalid filenames like screenshot-1234567890.svg+xml.
Update the extension logic to handle image/svg+xml correctly:
Proposed fix
let baseFilename: string;
if (options.isFromPaste || !file.name || file.name === 'image.png') {
- const extension = file.type.split('/')[1] || 'png';
+ const extension =
+ file.type === 'image/svg+xml'
+ ? 'svg'
+ : (file.type.split('/')[1] || 'png').replace(/\+.*/, '');
baseFilename = `screenshot-${Date.now()}.${extension}`;
} else {
baseFilename = file.name;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const dataUrl = await blobToBase64(file); | |
| const thumbnail = await createThumbnail(dataUrl); | |
| // Generate filename based on source | |
| let baseFilename: string; | |
| if (options.isFromPaste || !file.name || file.name === 'image.png') { | |
| const extension = file.type.split('/')[1] || 'png'; | |
| baseFilename = `screenshot-${Date.now()}.${extension}`; | |
| } else { | |
| baseFilename = file.name; | |
| } | |
| const resolvedFilename = resolveFilename(baseFilename, [ | |
| ...existingFilenames, | |
| ...newImages.map((img) => img.filename) | |
| ]); | |
| newImages.push({ | |
| id: generateImageId(), | |
| filename: resolvedFilename, | |
| mimeType: file.type, | |
| size: file.size, | |
| data: dataUrl.split(',')[1], // Store base64 without data URL prefix | |
| thumbnail | |
| }); | |
| } catch (error) { | |
| console.error('Image processing error:', error); | |
| onError?.(options.isFromPaste ? errors.processPasteFailed : errors.processDropFailed); | |
| } | |
| } | |
| try { | |
| const dataUrl = await blobToBase64(file); | |
| const thumbnail = await createThumbnail(dataUrl); | |
| // Generate filename based on source | |
| let baseFilename: string; | |
| if (options.isFromPaste || !file.name || file.name === 'image.png') { | |
| const extension = | |
| file.type === 'image/svg+xml' | |
| ? 'svg' | |
| : (file.type.split('/')[1] || 'png').replace(/\+.*/, ''); | |
| baseFilename = `screenshot-${Date.now()}.${extension}`; | |
| } else { | |
| baseFilename = file.name; | |
| } | |
| const resolvedFilename = resolveFilename(baseFilename, [ | |
| ...existingFilenames, | |
| ...newImages.map((img) => img.filename) | |
| ]); | |
| newImages.push({ | |
| id: generateImageId(), | |
| filename: resolvedFilename, | |
| mimeType: file.type, | |
| size: file.size, | |
| data: dataUrl.split(',')[1], // Store base64 without data URL prefix | |
| thumbnail | |
| }); | |
| } catch (error) { | |
| console.error('Image processing error:', error); | |
| onError?.(options.isFromPaste ? errors.processPasteFailed : errors.processDropFailed); | |
| } | |
| } |
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/task-form/useImageUpload.ts around
lines 140 - 170, When deriving the file extension from file.type in the image
processing block (around blobToBase64/createThumbnail and before
resolveFilename), sanitize the MIME subtype so SVGs and other +suffix types
produce correct extensions: get the subtype via file.type.split('/')[1], then if
it contains a '+' take the substring before the '+' and normalize to lowercase;
if the result is empty fall back to 'png'. Use that sanitized extension when
building baseFilename (used before resolveFilename and when pushing newImages
entries).
| // Fetch branches when dialog opens | ||
| useEffect(() => { | ||
| if (open && projectPath) { | ||
| fetchBranches(); | ||
| fetchProjectDefaultBranch(); | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [open, projectPath]); | ||
|
|
||
| const fetchBranches = async () => { | ||
| if (!projectPath) return; | ||
| let isMounted = true; | ||
|
|
||
| setIsLoadingBranches(true); | ||
| try { | ||
| const result = await window.electronAPI.getGitBranches(projectPath); | ||
| if (result.success && result.data) { | ||
| setBranches(result.data); | ||
| const fetchBranches = async () => { | ||
| if (!projectPath) return; | ||
| if (isMounted) setIsLoadingBranches(true); | ||
| try { | ||
| const result = await window.electronAPI.getGitBranches(projectPath); | ||
| if (isMounted && result.success && result.data) { | ||
| setBranches(result.data); | ||
| } | ||
| } catch (err) { | ||
| console.error('Failed to fetch branches:', err); | ||
| } finally { | ||
| if (isMounted) setIsLoadingBranches(false); | ||
| } | ||
| } catch (err) { | ||
| console.error('Failed to fetch branches:', err); | ||
| } finally { | ||
| setIsLoadingBranches(false); | ||
| } | ||
| }; | ||
| }; | ||
|
|
||
| const fetchProjectDefaultBranch = async () => { | ||
| if (!projectId) return; | ||
|
|
||
| try { | ||
| // Get env config to check if there's a configured default branch | ||
| const result = await window.electronAPI.getProjectEnv(projectId); | ||
| if (result.success && result.data?.defaultBranch) { | ||
| setProjectDefaultBranch(result.data.defaultBranch); | ||
| } else if (projectPath) { | ||
| // Fall back to auto-detect | ||
| const detectResult = await window.electronAPI.detectMainBranch(projectPath); | ||
| if (detectResult.success && detectResult.data) { | ||
| setProjectDefaultBranch(detectResult.data); | ||
| const fetchProjectDefaultBranch = async () => { | ||
| if (!projectId) return; | ||
| try { | ||
| const result = await window.electronAPI.getProjectEnv(projectId); | ||
| if (isMounted && result.success && result.data?.defaultBranch) { | ||
| setProjectDefaultBranch(result.data.defaultBranch); | ||
| } else if (projectPath) { | ||
| const detectResult = await window.electronAPI.detectMainBranch(projectPath); | ||
| if (isMounted && detectResult.success && detectResult.data) { | ||
| setProjectDefaultBranch(detectResult.data); | ||
| } | ||
| } | ||
| } catch (err) { | ||
| console.error('Failed to fetch project default branch:', err); | ||
| } | ||
| } catch (err) { | ||
| console.error('Failed to fetch project default branch:', err); | ||
| }; | ||
|
|
||
| if (open && projectPath) { | ||
| fetchBranches(); | ||
| fetchProjectDefaultBranch(); | ||
| } | ||
| }; | ||
|
|
||
| return () => { | ||
| isMounted = false; | ||
| }; | ||
| }, [open, projectPath, projectId]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and examine the relevant section
cat -n apps/frontend/src/renderer/components/TaskCreationWizard.tsx | sed -n '150,210p'Repository: AndyMik90/Auto-Claude
Length of output: 2569
🏁 Script executed:
# Check the full context of the useEffect hook
cat -n apps/frontend/src/renderer/components/TaskCreationWizard.tsx | sed -n '140,220p'Repository: AndyMik90/Auto-Claude
Length of output: 3281
🏁 Script executed:
# Also check for any hardcoded user-facing strings in this file that should use i18n
rg -n "setIsLoadingBranches|console\.error" apps/frontend/src/renderer/components/TaskCreationWizard.tsx -A 2 -B 2Repository: AndyMik90/Auto-Claude
Length of output: 968
🏁 Script executed:
# Check if there are other useEffect patterns with similar isMounted issues in the file
rg -n "let isMounted" apps/frontend/src/renderer/components/TaskCreationWizard.tsxRepository: AndyMik90/Auto-Claude
Length of output: 94
🏁 Script executed:
# Check for any hardcoded user-facing strings in the component that might violate i18n guidelines
cat apps/frontend/src/renderer/components/TaskCreationWizard.tsx | head -1Repository: AndyMik90/Auto-Claude
Length of output: 68
🏁 Script executed:
# Look for any hardcoded strings (error messages, labels) in the entire file
rg -n '["\'`].*["\'\`]' apps/frontend/src/renderer/components/TaskCreationWizard.tsx | grep -E '(Error|Failed|Branch|Project)' | head -20Repository: AndyMik90/Auto-Claude
Length of output: 133
🏁 Script executed:
# Check if the file uses useTranslation hook
rg -n "useTranslation|i18n" apps/frontend/src/renderer/components/TaskCreationWizard.tsxRepository: AndyMik90/Auto-Claude
Length of output: 171
Fix race condition where isLoadingBranches can remain stuck true after dialog closes.
When open flips to false before fetchBranches() settles, the cleanup function sets isMounted = false, and the finally block (line 171) won't reset isLoadingBranches because of the guard. This leaves the loading state stuck even though the dialog is closed.
Move the loading reset to the cleanup function (where it always runs) and keep the isActive guard only for data mutations:
Proposed fix
useEffect(() => {
- let isMounted = true;
+ let isActive = true;
const fetchBranches = async () => {
if (!projectPath) return;
- if (isMounted) setIsLoadingBranches(true);
+ setIsLoadingBranches(true);
try {
const result = await window.electronAPI.getGitBranches(projectPath);
- if (isMounted && result.success && result.data) {
+ if (isActive && result.success && result.data) {
setBranches(result.data);
}
} catch (err) {
console.error('Failed to fetch branches:', err);
} finally {
- if (isMounted) setIsLoadingBranches(false);
+ if (isActive) setIsLoadingBranches(false);
}
};
const fetchProjectDefaultBranch = async () => {
if (!projectId) return;
try {
const result = await window.electronAPI.getProjectEnv(projectId);
- if (isMounted && result.success && result.data?.defaultBranch) {
+ if (isActive && result.success && result.data?.defaultBranch) {
setProjectDefaultBranch(result.data.defaultBranch);
} else if (projectPath) {
const detectResult = await window.electronAPI.detectMainBranch(projectPath);
- if (isMounted && detectResult.success && detectResult.data) {
+ if (isActive && detectResult.success && detectResult.data) {
setProjectDefaultBranch(detectResult.data);
}
}
} catch (err) {
console.error('Failed to fetch project default branch:', err);
}
};
if (open && projectPath) {
fetchBranches();
fetchProjectDefaultBranch();
}
return () => {
- isMounted = false;
+ isActive = false;
+ setIsLoadingBranches(false);
};
}, [open, projectPath, projectId]);🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/TaskCreationWizard.tsx around lines
156 - 200, The useEffect fetchBranches flow can leave isLoadingBranches true if
the dialog closes before fetch finishes because the finally block is skipped by
the isMounted guard; update the effect so the cleanup always resets loading:
keep setting isLoadingBranches(true) at the start of fetchBranches, remove the
guarded setIsLoadingBranches(false) from the finally block, and instead call
setIsLoadingBranches(false) unconditionally inside the useEffect cleanup; retain
the isMounted/isActive checks only for mutating state like setBranches and
setProjectDefaultBranch in fetchBranches and fetchProjectDefaultBranch to avoid
setting state on unmounted components.
| const handleDescriptionChange = useCallback((newValue: string) => { | ||
| const textarea = descriptionRef.current; | ||
| const cursorPos = textarea?.selectionStart || 0; | ||
|
|
||
| setDescription(newValue); | ||
|
|
||
| // Check for @ mention at cursor | ||
| const mention = detectAtMention(newValue, cursorPos); | ||
|
|
||
| if (mention) { | ||
| // Calculate popup position based on cursor | ||
| const textarea = descriptionRef.current; | ||
| if (textarea) { | ||
| const rect = textarea.getBoundingClientRect(); | ||
| const textareaStyle = window.getComputedStyle(textarea); | ||
| const lineHeight = parseFloat(textareaStyle.lineHeight) || 20; | ||
| const paddingTop = parseFloat(textareaStyle.paddingTop) || 8; | ||
| const paddingLeft = parseFloat(textareaStyle.paddingLeft) || 12; | ||
|
|
||
| // Estimate cursor position (simplified - assumes fixed-width font) | ||
| const textBeforeCursor = newValue.slice(0, cursorPos); | ||
| const lines = textBeforeCursor.split('\n'); | ||
| const currentLineIndex = lines.length - 1; | ||
| const currentLineLength = lines[currentLineIndex].length; | ||
|
|
||
| // Calculate position relative to textarea | ||
| const charWidth = 8; // Approximate character width | ||
| const top = paddingTop + (currentLineIndex + 1) * lineHeight + 4; | ||
| const left = paddingLeft + Math.min(currentLineLength * charWidth, rect.width - 300); | ||
|
|
||
| setAutocomplete({ | ||
| show: true, | ||
| query: mention.query, | ||
| startPos: mention.startPos, | ||
| position: { top, left: Math.max(0, left) } | ||
| }); | ||
| } | ||
| } else { | ||
| // No @ mention at cursor, close autocomplete | ||
| if (autocomplete?.show) { | ||
| setAutocomplete(null); | ||
| } | ||
| if (mention && textarea) { | ||
| const rect = textarea.getBoundingClientRect(); | ||
| const textareaStyle = window.getComputedStyle(textarea); | ||
| const lineHeight = parseFloat(textareaStyle.lineHeight) || 20; | ||
| const paddingTop = parseFloat(textareaStyle.paddingTop) || 8; | ||
| const paddingLeft = parseFloat(textareaStyle.paddingLeft) || 12; | ||
|
|
||
| const textBeforeCursor = newValue.slice(0, cursorPos); | ||
| const lines = textBeforeCursor.split('\n'); | ||
| const currentLineIndex = lines.length - 1; | ||
| const currentLineLength = lines[currentLineIndex].length; | ||
|
|
||
| const charWidth = 8; | ||
| const top = paddingTop + (currentLineIndex + 1) * lineHeight + 4; | ||
| const left = paddingLeft + Math.min(currentLineLength * charWidth, rect.width - 300); | ||
|
|
||
| setAutocomplete({ | ||
| show: true, | ||
| query: mention.query, | ||
| startPos: mention.startPos, | ||
| position: { top, left: Math.max(0, left) } | ||
| }); | ||
| } else if (autocomplete?.show) { | ||
| setAutocomplete(null); | ||
| } | ||
| }, [detectAtMention, autocomplete?.show]); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd -t f "TaskCreationWizard.tsx"Repository: AndyMik90/Auto-Claude
Length of output: 125
🏁 Script executed:
fd -t f "FileAutocomplete" --extension tsx --extension tsRepository: AndyMik90/Auto-Claude
Length of output: 123
🏁 Script executed:
wc -l apps/frontend/src/renderer/components/TaskCreationWizard.tsxRepository: AndyMik90/Auto-Claude
Length of output: 129
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/TaskCreationWizard.tsx | sed -n '230,280p'Repository: AndyMik90/Auto-Claude
Length of output: 2175
🏁 Script executed:
wc -l apps/frontend/src/renderer/components/FileAutocomplete.tsxRepository: AndyMik90/Auto-Claude
Length of output: 127
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/FileAutocomplete.tsxRepository: AndyMik90/Auto-Claude
Length of output: 8889
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/TaskCreationWizard.tsx | grep -A 5 -B 5 "FileAutocomplete"Repository: AndyMik90/Auto-Claude
Length of output: 1427
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/TaskCreationWizard.tsx | sed -n '600,640p'Repository: AndyMik90/Auto-Claude
Length of output: 2107
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx | head -50Repository: AndyMik90/Auto-Claude
Length of output: 1873
🏁 Script executed:
rg "position:" apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsxRepository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/TaskCreationWizard.tsx | sed -n '540,570p'Repository: AndyMik90/Auto-Claude
Length of output: 1436
🏁 Script executed:
wc -l apps/frontend/src/renderer/components/task-form/TaskFormFields.tsxRepository: AndyMik90/Auto-Claude
Length of output: 135
🏁 Script executed:
rg "position" apps/frontend/src/renderer/components/task-form/TaskFormFields.tsxRepository: AndyMik90/Auto-Claude
Length of output: 161
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx | head -100Repository: AndyMik90/Auto-Claude
Length of output: 4161
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx | sed -n '120,200p'Repository: AndyMik90/Auto-Claude
Length of output: 3716
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx | sed -n '140,180p'Repository: AndyMik90/Auto-Claude
Length of output: 1792
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx | sed -n '320,347p'Repository: AndyMik90/Auto-Claude
Length of output: 1195
🏁 Script executed:
rg "children" apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx -A 2 -B 2Repository: AndyMik90/Auto-Claude
Length of output: 438
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx | sed -n '158,195p'Repository: AndyMik90/Auto-Claude
Length of output: 2021
🏁 Script executed:
rg "className.*relative" apps/frontend/src/renderer/components/TaskCreationWizard.tsxRepository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
rg "className.*absolute|position.*relative|position.*absolute" apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx | head -20Repository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx | sed -n '158,275p'Repository: AndyMik90/Auto-Claude
Length of output: 5756
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/TaskCreationWizard.tsx | sed -n '520,570p'Repository: AndyMik90/Auto-Claude
Length of output: 2423
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/TaskCreationWizard.tsx | sed -n '500,520p'Repository: AndyMik90/Auto-Claude
Length of output: 882
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx | sed -n '40,100p'Repository: AndyMik90/Auto-Claude
Length of output: 2761
🏁 Script executed:
cat -n apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx | sed -n '100,150p'Repository: AndyMik90/Auto-Claude
Length of output: 1642
🏁 Script executed:
python3 << 'EOF'
# Analyze the positioning context
analysis = {
"FileAutocomplete_rendering": "Rendered as child of TaskFormFields (line 235 in TaskFormFields.tsx)",
"FileAutocomplete_position": "absolute z-50 with inline style top/left",
"TaskFormFields_structure": {
"outer_div": "className='space-y-6' (NO position)",
"description_wrapper": "className='space-y-2' (NO position)",
"textarea_container": "className='relative' (HAS position:relative)",
"children_location": "OUTSIDE textarea_container, inside outer div"
},
"positioning_hierarchy": [
"DialogPrimitive.Content (fixed left-[50%] top-4 z-50) - POSITIONED",
"Main content area divs",
"TaskCreationWizard div (space-y-6)",
"TaskFormFields div (space-y-6) - NO position",
"Description section div (space-y-2) - NO position",
"Textarea container div (relative) - HAS position:relative",
"FileAutocomplete (absolute) - positioned relative to NEAREST positioned ancestor"
]
}
# The FileAutocomplete is rendered AFTER the relative div closes,
# so it's not a child of the relative div
print("KEY FINDING:")
print("FileAutocomplete is NOT a child of the 'relative' div")
print("It's a sibling of the 'space-y-2' section div")
print("Nearest positioned ancestor: DialogPrimitive.Content (fixed)")
print()
print("This means:")
print("- Coordinates calculated relative to textarea content (content-space)")
print("- Applied as absolute positioning relative to modal (viewport-space)")
print("- Result: Severely misaligned popup")
EOFRepository: AndyMik90/Auto-Claude
Length of output: 151
Autocomplete popup positioning is broken (incorrect coordinate system and missing offsets).
The coordinates are calculated as textarea-content-relative (line number × lineHeight + padding) but applied as absolute positioning relative to the fixed modal, causing severe misalignment. Additionally, the calculation is missing:
rect.top/rect.leftto convert from textarea element space to viewport spacetextarea.scrollTop/scrollLeftto account for scrolling within the textarea
The popup will not appear near the cursor. Fix by either:
- Computing viewport-relative coordinates (add
rect.top+rect.left+ account forscrollTop/scrollLeft), or - Repositioning
FileAutocompleteas a child of the textarea's immediate positioned ancestor and using content-relative coordinates
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/TaskCreationWizard.tsx around lines
239 - 272, handleDescriptionChange computes popup coords in textarea-local space
but applies them as absolute/viewport coords; update the positioning to convert
to viewport coordinates by adding the textarea bounding rect offsets and
subtracting textarea scroll offsets before calling setAutocomplete.
Specifically, in the handleDescriptionChange closure (uses descriptionRef,
detectAtMention and setAutocomplete) compute top = rect.top + paddingTop +
currentLineIndex * lineHeight + extraOffset - textarea.scrollTop and left =
rect.left + paddingLeft + Math.min(currentLineLength * charWidth, rect.width -
popupWidth) - textarea.scrollLeft, then pass those values into
setAutocomplete.position (clamped with Math.max(0, ...)). Alternatively, if you
prefer content-relative placement, render FileAutocomplete inside the textarea's
positioned ancestor and keep current content-relative math; ensure you handle
rect.top/left and textarea.scrollTop/scrollLeft in whichever approach you
choose.
AndyMik90
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 Auto Claude PR Review
🟠 Follow-up Review: Needs Revision
🟠 Needs revision - 1 blocking issue(s) require fixes.
Resolution Status
- ✅ Resolved: 2 previous findings addressed
- ❌ Unresolved: 0 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
🚨 Blocking Issues
- quality: Incomplete change detection - phaseModels and phaseThinking changes not detected
Verdict
⏳ CI Status: 2 checks pending - cannot approve until CI completes. ✅ Previous Findings: Both CMT-PERF-001 (performance) and CMT-QUALITY-001 (memory leak) are properly RESOLVED with correct implementations (useMemo for errors object, isMounted pattern for async effects). 🆕 New Issues: 1 MEDIUM severity issue found - TaskEditDialog's hasChanges check doesn't include phaseModels/phaseThinking, causing potential silent data loss when users only modify phase configuration. The LOW severity incomplete useWorktree UI is a minor polish item. Action Required: (1) Wait for CI checks to pass, (2) Fix the hasChanges logic in TaskEditDialog to include phaseModels and phaseThinking comparisons.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (2 selected of 2 total)
🟡 [NEW-001] [MEDIUM] Incomplete change detection - phaseModels and phaseThinking changes not detected
📁 apps/frontend/src/renderer/components/TaskEditDialog.tsx:175
The hasChanges check in handleSave does not include phaseModels or phaseThinking state variables, even though these values can be changed via the TaskFormFields component. If a user only changes the phase model configuration, the dialog will close without saving because hasChanges will be false.
Suggested fix:
Add phaseModels and phaseThinking to the hasChanges check using JSON.stringify comparison
🔵 [NEW-002] [LOW] Incomplete feature - useWorktree checkbox UI not rendered
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:76
The useWorktree state is defined and used in task creation metadata, and translation keys exist (useWorktreeLabel, useWorktreeDescription), but no checkbox or toggle is rendered in the Git Options section to allow users to change this value. State defaults to true.
Suggested fix:
Either add the Checkbox UI in the Git Options section or remove unused state and translation keys if feature is intentionally hidden
This review was generated by Auto Claude.
TaskEditDialog's hasChanges logic was missing phaseModels and phaseThinking comparisons, which could cause silent data loss when users only modified phase configuration without changing other fields. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Fourth Round - Change Detection Fix ✅Fixed the blocking issue for incomplete change detection: Incomplete hasChanges Check ✅Fixed in Added const hasChanges =
trimmedTitle !== task.title ||
trimmedDescription !== task.description ||
// ... other existing checks ...
JSON.stringify(images) !== JSON.stringify(task.metadata?.attachedImages || []) ||
// NEW: Include phase configuration in change detection
JSON.stringify(phaseModels) !== JSON.stringify(task.metadata?.phaseModels || DEFAULT_PHASE_MODELS) ||
JSON.stringify(phaseThinking) !== JSON.stringify(task.metadata?.phaseThinking || DEFAULT_PHASE_THINKING);This ensures that changes to phase-specific model and thinking level configurations are properly detected and saved. All blocking issues from the four review rounds have been addressed. Waiting for CI to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @apps/frontend/src/renderer/components/TaskEditDialog.tsx:
- Around line 232-295: The inline arrow passed to TaskFormFields via the
onProfileChange prop creates a new function on every render; extract it into a
stable handler (e.g., handleProfileChange) using React.useCallback and pass that
instead. Implement handleProfileChange to call setProfileId, setModel and
setThinkingLevel and include the proper dependency array (the state setters or
none if they come from useState), then replace the inline function in the
onProfileChange prop with handleProfileChange to avoid unnecessary re-renders.
- Around line 186-187: The hasChanges comparison uses
JSON.stringify(phaseModels) !== JSON.stringify(task.metadata?.phaseModels ||
DEFAULT_PHASE_MODELS) and a similar line for phaseThinking but omits the
selectedProfile fallback used when initializing phaseModels/phaseThinking;
update those comparisons to mirror the initial fallback chain
(task.metadata?.phaseModels || selectedProfile.phaseModels ||
DEFAULT_PHASE_MODELS and task.metadata?.phaseThinking ||
selectedProfile.phaseThinking || DEFAULT_PHASE_THINKING) so the dialog only
flags real edits.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/renderer/components/TaskEditDialog.tsx
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/TaskEditDialog.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/TaskEditDialog.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/TaskEditDialog.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:48.743Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/renderer/components/TaskEditDialog.tsx
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings) can be deferred for future i18n cleanup passes. Do not fix such issues in PRs that do not introduce new i18n violations, especially in frontend TSX components (e.g., apps/frontend/**/*.tsx). If a PR adds new i18n violations, address them in that PR.
Applied to files:
apps/frontend/src/renderer/components/TaskEditDialog.tsx
🧬 Code graph analysis (1)
apps/frontend/src/renderer/components/TaskEditDialog.tsx (2)
apps/frontend/src/renderer/components/task-form/TaskModalLayout.tsx (1)
TaskModalLayout(40-139)apps/frontend/src/renderer/components/task-form/TaskFormFields.tsx (1)
TaskFormFields(92-347)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (2)
apps/frontend/src/renderer/components/TaskEditDialog.tsx (2)
29-35: LGTM on imports and component structure.The imports are minimal and appropriate for the refactored component. The switch from inline Dialog components to shared
TaskModalLayoutandTaskFormFieldsaligns well with the PR objective of reducing code duplication and ensuring consistent modal sizing.
210-211: Good fix for image removal persistence.Always setting
attachedImages(even to an empty array) ensures that removing all images is properly persisted. This addresses the bug mentioned in PR comments.
| JSON.stringify(phaseModels) !== JSON.stringify(task.metadata?.phaseModels || DEFAULT_PHASE_MODELS) || | ||
| JSON.stringify(phaseThinking) !== JSON.stringify(task.metadata?.phaseThinking || DEFAULT_PHASE_THINKING); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent fallback in hasChanges comparison for phase configs.
The initial state for phaseModels (line 100) uses a three-level fallback: task.metadata?.phaseModels || selectedProfile.phaseModels || DEFAULT_PHASE_MODELS. However, the hasChanges comparison here only falls back to DEFAULT_PHASE_MODELS, skipping selectedProfile.phaseModels.
This could cause false positives where the dialog detects "changes" when the user hasn't actually modified anything, if selectedProfile.phaseModels differs from DEFAULT_PHASE_MODELS.
Suggested fix to align fallback chains
- JSON.stringify(phaseModels) !== JSON.stringify(task.metadata?.phaseModels || DEFAULT_PHASE_MODELS) ||
- JSON.stringify(phaseThinking) !== JSON.stringify(task.metadata?.phaseThinking || DEFAULT_PHASE_THINKING);
+ JSON.stringify(phaseModels) !== JSON.stringify(task.metadata?.phaseModels || selectedProfile.phaseModels || DEFAULT_PHASE_MODELS) ||
+ JSON.stringify(phaseThinking) !== JSON.stringify(task.metadata?.phaseThinking || selectedProfile.phaseThinking || DEFAULT_PHASE_THINKING);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| JSON.stringify(phaseModels) !== JSON.stringify(task.metadata?.phaseModels || DEFAULT_PHASE_MODELS) || | |
| JSON.stringify(phaseThinking) !== JSON.stringify(task.metadata?.phaseThinking || DEFAULT_PHASE_THINKING); | |
| JSON.stringify(phaseModels) !== JSON.stringify(task.metadata?.phaseModels || selectedProfile.phaseModels || DEFAULT_PHASE_MODELS) || | |
| JSON.stringify(phaseThinking) !== JSON.stringify(task.metadata?.phaseThinking || selectedProfile.phaseThinking || DEFAULT_PHASE_THINKING); |
🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/TaskEditDialog.tsx around lines 186 -
187, The hasChanges comparison uses JSON.stringify(phaseModels) !==
JSON.stringify(task.metadata?.phaseModels || DEFAULT_PHASE_MODELS) and a similar
line for phaseThinking but omits the selectedProfile fallback used when
initializing phaseModels/phaseThinking; update those comparisons to mirror the
initial fallback chain (task.metadata?.phaseModels ||
selectedProfile.phaseModels || DEFAULT_PHASE_MODELS and
task.metadata?.phaseThinking || selectedProfile.phaseThinking ||
DEFAULT_PHASE_THINKING) so the dialog only flags real edits.
| return ( | ||
| <Dialog open={open} onOpenChange={handleClose}> | ||
| <DialogContent className="sm:max-w-[550px] max-h-[90vh] overflow-y-auto"> | ||
| <DialogHeader> | ||
| <DialogTitle className="text-foreground">Edit Task</DialogTitle> | ||
| <DialogDescription> | ||
| Update task details including title, description, classification, images, and settings. Changes will be saved to the spec files. | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
|
|
||
| <div className="space-y-5 py-4"> | ||
| {/* Description (Primary - Required) */} | ||
| <div className="space-y-2"> | ||
| <Label htmlFor="edit-description" className="text-sm font-medium text-foreground"> | ||
| Description <span className="text-destructive">*</span> | ||
| </Label> | ||
| <Textarea | ||
| ref={descriptionRef} | ||
| id="edit-description" | ||
| placeholder="Describe the feature, bug fix, or improvement. Be as specific as possible about requirements, constraints, and expected behavior." | ||
| value={description} | ||
| onChange={(e) => setDescription(e.target.value)} | ||
| onPaste={handlePaste} | ||
| onDragOver={handleTextareaDragOver} | ||
| onDragLeave={handleTextareaDragLeave} | ||
| onDrop={handleTextareaDrop} | ||
| rows={5} | ||
| disabled={isSaving} | ||
| aria-required="true" | ||
| aria-describedby="edit-description-help" | ||
| className={cn( | ||
| isDragOverTextarea && !isSaving && "border-primary bg-primary/5 ring-2 ring-primary/20" | ||
| )} | ||
| /> | ||
| <p id="edit-description-help" className="text-xs text-muted-foreground"> | ||
| {t('images.pasteHint', { shortcut: navigator.platform.includes('Mac') ? '⌘V' : 'Ctrl+V' })} | ||
| </p> | ||
| </div> | ||
|
|
||
| {/* Title (Optional - Auto-generated if empty) */} | ||
| <div className="space-y-2"> | ||
| <Label htmlFor="edit-title" className="text-sm font-medium text-foreground"> | ||
| Task Title <span className="text-muted-foreground font-normal">(optional)</span> | ||
| </Label> | ||
| <Input | ||
| id="edit-title" | ||
| placeholder="Leave empty to auto-generate from description" | ||
| value={title} | ||
| onChange={(e) => setTitle(e.target.value)} | ||
| disabled={isSaving} | ||
| /> | ||
| <p className="text-xs text-muted-foreground"> | ||
| A short, descriptive title will be generated automatically if left empty. | ||
| </p> | ||
| </div> | ||
|
|
||
| {/* Agent Profile Selection */} | ||
| <AgentProfileSelector | ||
| profileId={profileId} | ||
| model={model} | ||
| thinkingLevel={thinkingLevel} | ||
| phaseModels={phaseModels} | ||
| phaseThinking={phaseThinking} | ||
| onProfileChange={(newProfileId, newModel, newThinkingLevel) => { | ||
| setProfileId(newProfileId); | ||
| setModel(newModel); | ||
| setThinkingLevel(newThinkingLevel); | ||
| }} | ||
| onModelChange={setModel} | ||
| onThinkingLevelChange={setThinkingLevel} | ||
| onPhaseModelsChange={setPhaseModels} | ||
| onPhaseThinkingChange={setPhaseThinking} | ||
| disabled={isSaving} | ||
| /> | ||
|
|
||
| {/* Paste Success Indicator */} | ||
| {pasteSuccess && ( | ||
| <div className="flex items-center gap-2 text-sm text-success animate-in fade-in slide-in-from-top-1 duration-200"> | ||
| <ImageIcon className="h-4 w-4" /> | ||
| Image added successfully! | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Advanced Options Toggle */} | ||
| <button | ||
| type="button" | ||
| onClick={() => setShowAdvanced(!showAdvanced)} | ||
| className={cn( | ||
| 'flex items-center gap-2 text-sm text-muted-foreground hover:text-foreground transition-colors', | ||
| 'w-full justify-between py-2 px-3 rounded-md hover:bg-muted/50' | ||
| )} | ||
| disabled={isSaving} | ||
| aria-expanded={showAdvanced} | ||
| aria-controls="edit-advanced-options" | ||
| > | ||
| <span>Classification (optional)</span> | ||
| {showAdvanced ? ( | ||
| <ChevronUp className="h-4 w-4" /> | ||
| ) : ( | ||
| <ChevronDown className="h-4 w-4" /> | ||
| )} | ||
| </button> | ||
|
|
||
| {/* Advanced Options */} | ||
| {showAdvanced && ( | ||
| <div id="edit-advanced-options" className="space-y-4 p-4 rounded-lg border border-border bg-muted/30"> | ||
| <div className="grid grid-cols-2 gap-4"> | ||
| {/* Category */} | ||
| <div className="space-y-2"> | ||
| <Label htmlFor="edit-category" className="text-xs font-medium text-muted-foreground"> | ||
| Category | ||
| </Label> | ||
| <Select | ||
| value={category} | ||
| onValueChange={(value) => setCategory(value as TaskCategory)} | ||
| disabled={isSaving} | ||
| > | ||
| <SelectTrigger id="edit-category" className="h-9"> | ||
| <SelectValue placeholder="Select category" /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| {Object.entries(TASK_CATEGORY_LABELS).map(([value, label]) => ( | ||
| <SelectItem key={value} value={value}> | ||
| {label} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| </div> | ||
|
|
||
| {/* Priority */} | ||
| <div className="space-y-2"> | ||
| <Label htmlFor="edit-priority" className="text-xs font-medium text-muted-foreground"> | ||
| Priority | ||
| </Label> | ||
| <Select | ||
| value={priority} | ||
| onValueChange={(value) => setPriority(value as TaskPriority)} | ||
| disabled={isSaving} | ||
| > | ||
| <SelectTrigger id="edit-priority" className="h-9"> | ||
| <SelectValue placeholder="Select priority" /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| {Object.entries(TASK_PRIORITY_LABELS).map(([value, label]) => ( | ||
| <SelectItem key={value} value={value}> | ||
| {label} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| </div> | ||
|
|
||
| {/* Complexity */} | ||
| <div className="space-y-2"> | ||
| <Label htmlFor="edit-complexity" className="text-xs font-medium text-muted-foreground"> | ||
| Complexity | ||
| </Label> | ||
| <Select | ||
| value={complexity} | ||
| onValueChange={(value) => setComplexity(value as TaskComplexity)} | ||
| disabled={isSaving} | ||
| > | ||
| <SelectTrigger id="edit-complexity" className="h-9"> | ||
| <SelectValue placeholder="Select complexity" /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| {Object.entries(TASK_COMPLEXITY_LABELS).map(([value, label]) => ( | ||
| <SelectItem key={value} value={value}> | ||
| {label} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| </div> | ||
|
|
||
| {/* Impact */} | ||
| <div className="space-y-2"> | ||
| <Label htmlFor="edit-impact" className="text-xs font-medium text-muted-foreground"> | ||
| Impact | ||
| </Label> | ||
| <Select | ||
| value={impact} | ||
| onValueChange={(value) => setImpact(value as TaskImpact)} | ||
| disabled={isSaving} | ||
| > | ||
| <SelectTrigger id="edit-impact" className="h-9"> | ||
| <SelectValue placeholder="Select impact" /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| {Object.entries(TASK_IMPACT_LABELS).map(([value, label]) => ( | ||
| <SelectItem key={value} value={value}> | ||
| {label} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| </div> | ||
| </div> | ||
|
|
||
| <p className="text-xs text-muted-foreground"> | ||
| These labels help organize and prioritize tasks. They're optional but useful for filtering. | ||
| </p> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Images Toggle */} | ||
| <button | ||
| type="button" | ||
| onClick={() => setShowImages(!showImages)} | ||
| className={cn( | ||
| 'flex items-center gap-2 text-sm text-muted-foreground hover:text-foreground transition-colors', | ||
| 'w-full justify-between py-2 px-3 rounded-md hover:bg-muted/50' | ||
| )} | ||
| disabled={isSaving} | ||
| aria-expanded={showImages} | ||
| aria-controls="edit-images-section" | ||
| > | ||
| <span className="flex items-center gap-2"> | ||
| <ImageIcon className="h-4 w-4" /> | ||
| Reference Images (optional) | ||
| {images.length > 0 && ( | ||
| <span className="text-xs bg-primary/10 text-primary px-1.5 py-0.5 rounded"> | ||
| {images.length} | ||
| </span> | ||
| )} | ||
| </span> | ||
| {showImages ? ( | ||
| <ChevronUp className="h-4 w-4" /> | ||
| ) : ( | ||
| <ChevronDown className="h-4 w-4" /> | ||
| )} | ||
| </button> | ||
|
|
||
| {/* Image Upload Section */} | ||
| {showImages && ( | ||
| <div id="edit-images-section" className="space-y-3 p-4 rounded-lg border border-border bg-muted/30"> | ||
| <p className="text-xs text-muted-foreground"> | ||
| Attach screenshots, mockups, or diagrams to provide visual context for the AI. | ||
| </p> | ||
| <ImageUpload | ||
| images={images} | ||
| onImagesChange={setImages} | ||
| disabled={isSaving} | ||
| /> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Review Requirement Toggle */} | ||
| <div className="flex items-start gap-3 p-4 rounded-lg border border-border bg-muted/30"> | ||
| <Checkbox | ||
| id="edit-require-review" | ||
| checked={requireReviewBeforeCoding} | ||
| onCheckedChange={(checked) => setRequireReviewBeforeCoding(checked === true)} | ||
| disabled={isSaving} | ||
| className="mt-0.5" | ||
| /> | ||
| <div className="flex-1 space-y-1"> | ||
| <Label | ||
| htmlFor="edit-require-review" | ||
| className="text-sm font-medium text-foreground cursor-pointer" | ||
| > | ||
| Require human review before coding | ||
| </Label> | ||
| <p className="text-xs text-muted-foreground"> | ||
| When enabled, you'll be prompted to review the spec and implementation plan before the coding phase begins. This allows you to approve, request changes, or provide feedback. | ||
| </p> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Error */} | ||
| {error && ( | ||
| <div className="flex items-start gap-2 rounded-lg bg-destructive/10 border border-destructive/30 p-3 text-sm text-destructive" role="alert"> | ||
| <X className="h-4 w-4 mt-0.5 shrink-0" /> | ||
| <span>{error}</span> | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
||
| <DialogFooter> | ||
| <Button variant="outline" onClick={handleClose} disabled={isSaving}> | ||
| Cancel | ||
| <TaskModalLayout | ||
| open={open} | ||
| onOpenChange={onOpenChange} | ||
| title={t('tasks:edit.title')} | ||
| description={t('tasks:edit.description')} | ||
| disabled={isSaving} | ||
| footer={ | ||
| <div className="flex items-center justify-end gap-3"> | ||
| <Button variant="outline" onClick={() => onOpenChange(false)} disabled={isSaving}> | ||
| {t('common:buttons.cancel')} | ||
| </Button> | ||
| <Button | ||
| onClick={handleSave} | ||
| disabled={isSaving || !isValid} | ||
| > | ||
| <Button onClick={handleSave} disabled={isSaving || !isValid}> | ||
| {isSaving ? ( | ||
| <> | ||
| <Loader2 className="mr-2 h-4 w-4 animate-spin" /> | ||
| Saving... | ||
| {t('common:buttons.saving')} | ||
| </> | ||
| ) : ( | ||
| 'Save Changes' | ||
| t('tasks:edit.saveChanges') | ||
| )} | ||
| </Button> | ||
| </DialogFooter> | ||
| </DialogContent> | ||
| </Dialog> | ||
| </div> | ||
| } | ||
| > | ||
| <TaskFormFields | ||
| description={description} | ||
| onDescriptionChange={setDescription} | ||
| title={title} | ||
| onTitleChange={setTitle} | ||
| profileId={profileId} | ||
| model={model} | ||
| thinkingLevel={thinkingLevel} | ||
| phaseModels={phaseModels} | ||
| phaseThinking={phaseThinking} | ||
| onProfileChange={(newProfileId, newModel, newThinkingLevel) => { | ||
| setProfileId(newProfileId); | ||
| setModel(newModel); | ||
| setThinkingLevel(newThinkingLevel); | ||
| }} | ||
| onModelChange={setModel} | ||
| onThinkingLevelChange={setThinkingLevel} | ||
| onPhaseModelsChange={setPhaseModels} | ||
| onPhaseThinkingChange={setPhaseThinking} | ||
| category={category} | ||
| priority={priority} | ||
| complexity={complexity} | ||
| impact={impact} | ||
| onCategoryChange={setCategory} | ||
| onPriorityChange={setPriority} | ||
| onComplexityChange={setComplexity} | ||
| onImpactChange={setImpact} | ||
| showClassification={showClassification} | ||
| onShowClassificationChange={setShowClassification} | ||
| images={images} | ||
| onImagesChange={setImages} | ||
| requireReviewBeforeCoding={requireReviewBeforeCoding} | ||
| onRequireReviewChange={setRequireReviewBeforeCoding} | ||
| disabled={isSaving} | ||
| error={error} | ||
| onError={setError} | ||
| idPrefix="edit" | ||
| /> | ||
| </TaskModalLayout> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Well-structured modal composition with proper i18n usage.
The refactored JSX cleanly delegates rendering to TaskModalLayout and TaskFormFields, achieving the PR's goal of consistent modal sizing and shared form components. All translation keys correctly use namespace prefixes (tasks:edit.*, common:buttons.*) as per coding guidelines.
Optional: The inline callback at lines 267-271 creates a new function reference on each render. If re-render performance becomes a concern, consider extracting it to a useCallback:
const handleProfileChange = useCallback((newProfileId: string, newModel: ModelType | '', newThinkingLevel: ThinkingLevel | '') => {
setProfileId(newProfileId);
setModel(newModel);
setThinkingLevel(newThinkingLevel);
}, []);🤖 Prompt for AI Agents
In @apps/frontend/src/renderer/components/TaskEditDialog.tsx around lines 232 -
295, The inline arrow passed to TaskFormFields via the onProfileChange prop
creates a new function on every render; extract it into a stable handler (e.g.,
handleProfileChange) using React.useCallback and pass that instead. Implement
handleProfileChange to call setProfileId, setModel and setThinkingLevel and
include the proper dependency array (the state setters or none if they come from
useState), then replace the inline function in the onProfileChange prop with
handleProfileChange to avoid unnecessary re-renders.
| JSON.stringify(images) !== JSON.stringify(task.metadata?.attachedImages || []); | ||
| JSON.stringify(images) !== JSON.stringify(task.metadata?.attachedImages || []) || | ||
| JSON.stringify(phaseModels) !== JSON.stringify(task.metadata?.phaseModels || DEFAULT_PHASE_MODELS) || | ||
| JSON.stringify(phaseThinking) !== JSON.stringify(task.metadata?.phaseThinking || DEFAULT_PHASE_THINKING); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent phaseModels comparison causes false change detection
Medium Severity
The hasChanges comparison uses task.metadata?.phaseModels || DEFAULT_PHASE_MODELS as the baseline, but the state initialization in the else branch (lines 144-150) uses selectedProfile.phaseModels || DEFAULT_PHASE_MODELS. When editing a task without phaseModels metadata while having a profile like 'auto' selected (which has AUTO_PHASE_MODELS different from DEFAULT_PHASE_MODELS), the comparison incorrectly reports changes. This causes the Save button to be enabled and triggers an update when the user made no actual changes.
Additional Locations (1)
| JSON.stringify(images) !== JSON.stringify(task.metadata?.attachedImages || []); | ||
| JSON.stringify(images) !== JSON.stringify(task.metadata?.attachedImages || []) || | ||
| JSON.stringify(phaseModels) !== JSON.stringify(task.metadata?.phaseModels || DEFAULT_PHASE_MODELS) || | ||
| JSON.stringify(phaseThinking) !== JSON.stringify(task.metadata?.phaseThinking || DEFAULT_PHASE_THINKING); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent fallback causes false positive change detection
Medium Severity
The hasChanges comparison for phaseModels and phaseThinking uses a different fallback chain than the state initialization. The initialization at line 148 uses selectedProfile.phaseModels || DEFAULT_PHASE_MODELS, but the comparison at line 186 uses task.metadata?.phaseModels || DEFAULT_PHASE_MODELS. When a task has no stored phaseModels and the user's selectedProfile has custom values differing from DEFAULT_PHASE_MODELS, the dialog incorrectly detects changes even when the user made none. This could cause unintended saves that add phase configuration to tasks.
Additional Locations (1)
AndyMik90
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 Auto Claude PR Review
🟠 Follow-up Review: Needs Revision
🟠 Needs revision - 1 unresolved finding(s) from previous review.
Resolution Status
- ✅ Resolved: 1 previous findings addressed
- ❌ Unresolved: 1 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 0 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 1 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
🚨 Blocking Issues
- quality: phaseModels and phaseThinking reset to defaults for non-autoProfile tasks
Verdict
✅ CI Status: All 17 checks passing. ✅ NEW-001 Resolved: phaseModels and phaseThinking now included in hasChanges check.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (3 selected of 3 total)
🔵 [NEW-002] [LOW] [UNRESOLVED] Incomplete feature - useWorktree checkbox UI not rendered
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:76
The useWorktree state is defined and used in task creation metadata, and translation keys exist (useWorktreeLabel, useWorktreeDescription), but no checkbox or toggle is rendered in the Git Options section to allow users to change this value. State defaults to true.
Resolution note: const [useWorktree, setUseWorktree] = useState(true); // line 76
// Git Options section (lines 599-636) only contains baseBranch Select - no useWorktree checkbox/toggle rendered
Suggested fix:
Either add the Checkbox UI in the Git Options section or remove unused state and translation keys if feature is intentionally hidden
🟡 [NEW-003] [MEDIUM] phaseModels and phaseThinking reset to defaults for non-autoProfile tasks
📁 apps/frontend/src/renderer/components/TaskEditDialog.tsx:142
In the useEffect that resets form state when the dialog opens (lines 135-150), when a task has model and thinkingLevel but is not an autoProfile, the code sets phaseModels and phaseThinking to DEFAULT_PHASE_MODELS and DEFAULT_PHASE_THINKING respectively, ignoring any custom phase configurations in task.metadata. This means editing a task with custom phase configurations will show default values instead of the task's actual values, causing data loss when saved.
Suggested fix:
Change lines 142-143 to:
setPhaseModels(task.metadata?.phaseModels || DEFAULT_PHASE_MODELS);
setPhaseThinking(task.metadata?.phaseThinking || DEFAULT_PHASE_THINKING);
🔵 [NEW-004] [LOW] errorMessages object prop causes unnecessary re-renders
📁 apps/frontend/src/renderer/components/task-form/useImageUpload.ts:82
The useMemo for the errors object has errorMessages in its dependency array. Since errorMessages is an object passed as a prop, it will be a new reference on every parent render unless the parent memoizes it. This means the errors memoization provides limited benefit and callbacks depending on it will be recreated each render.
Suggested fix:
Consider extracting individual error message strings from the prop in the parent component and passing them individually, or document that callers should memoize the errorMessages object.
This review was generated by Auto Claude.
…ile tasks When editing a task with custom model/thinkingLevel that isn't an autoProfile, the dialog was resetting phaseModels and phaseThinking to defaults instead of preserving the task's actual values from metadata. This could cause data loss. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
AndyMik90
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Auto Claude Review - APPROVED
Status: Ready to Merge
Summary: ## 🟡 Follow-up Review: Merge With Changes
🟡 Can merge - Minor suggestions noted, no blockers.
Resolution Status
- ✅ Resolved: 2 previous findings addressed
- ❌ Unresolved: 1 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 1 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 1 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
Verdict
✅ CI Status: All 17 checks passing.\n\n✅ NEW-003 RESOLVED: The main blocking issue (phaseModels and phaseThinking reset for non-autoProfile tasks) has been fixed in commit 78bd2fd. The useEffect now correctly preserves task.metadata values.\n\n
Review Process
Agents invoked: resolution-verifier, new-code-reviewer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
💡 Suggestions (3)
These are non-blocking suggestions for consideration:
🔵 [NEW-002] [LOW] [UNRESOLVED] [UNRESOLVED] Incomplete feature - useWorktree checkbox UI not rendered
📁 apps/frontend/src/renderer/components/TaskCreationWizard.tsx:76
The useWorktree state is defined and used in task creation metadata, and translation keys exist (useWorktreeLabel, useWorktreeDescription), but no checkbox or toggle is rendered in the Git Options section to allow users to change this value. State defaults to true.
Resolution note: const [useWorktree, setUseWorktree] = useState(true); // line 76
// Git Options section (lines 599-636) only contains baseBranch Select - no useWorktree checkbox/toggle rendered
Resolution note: // Line 76: State is defined
const [useWorktree, setUseWorktree] = useState(true);
// Lines 599-636: Git Options section - only contains base branch selector
{showGitOptions && (
Suggested fix:
Either add the Checkbox UI in the Git Options section or remove unused state and translation keys if feature is intentionally hidden
🔵 [NCR-001] [LOW] hasChanges check does not detect isAutoProfile changes
📁 apps/frontend/src/renderer/components/TaskEditDialog.tsx:175
The hasChanges check compares model, thinkingLevel, phaseModels, and phaseThinking, but does not compare whether isAutoProfile has changed. If a user switches between auto/non-auto profiles with identical configurations, the change won't be detected.
Suggested fix:
Add isAutoProfile comparison to hasChanges: (profileId === 'auto') !== (task.metadata?.isAutoProfile ?? false) ||
🔵 [NCR-002] [LOW] Classification Select components lack clear option
📁 apps/frontend/src/renderer/components/task-form/ClassificationFields.tsx:70
Once a user selects a classification value (category, priority, complexity, impact), there is no UI option to clear the selection back to empty. Users cannot unset a classification they previously set.
Suggested fix:
Add a 'None' SelectItem at the beginning of each SelectContent and add translation key 'form.classification.none'
This automated review found no blocking issues. The PR can be safely merged.
Generated by Auto Claude
AndyMik90#765) * refactor(ui): extract shared task form components for consistent modal sizing Create shared components to unify TaskCreationWizard, TaskEditDialog, and TaskDetailModal with consistent full-height modal sizing. New shared components in task-form/: - TaskModalLayout: Full-height modal matching TaskDetailModal (95vw, max-w-5xl) - TaskFormFields: Common form fields (description, title, profile, classification) - ClassificationFields: Task classification 2x2 grid dropdowns - useImageUpload: Hook for image paste/drop handling Benefits: - All 3 task modals now have identical dimensions and positioning - Reduced code duplication (1,938 → 1,651 lines total) - TaskCreationWizard: 1,176 → 623 lines (47% reduction) - TaskEditDialog: 762 → 293 lines (62% reduction) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: address PR review feedback for task form components - Fix HIGH: Pass descriptionRef from TaskCreationWizard to TaskFormFields to fix broken @ mention autocomplete positioning - Fix MEDIUM: Add i18n translations for all hardcoded strings in: - ClassificationFields.tsx - TaskFormFields.tsx - TaskModalLayout.tsx - TaskCreationWizard.tsx (modal, draft, buttons, git options) - TaskEditDialog.tsx - Fix MEDIUM: Correct isAutoProfile logic to only set true when profileId === 'auto' (not for all profiles with phase configs) - Fix MEDIUM: Update handleAutocompleteSelect signature to accept optional fullPath parameter - Fix LOW: Add proper setTimeout cleanup in useImageUpload.ts - Fix LOW: Use queueMicrotask instead of setTimeout in handleAutocompleteSelect for cursor position restoration - Fix LOW: Move fetch functions inside useEffect to fix exhaustive-deps warning - Add English and French translations for all new i18n keys 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: address all i18n violations and logic bug in task form components i18n Fixes: - Replace hardcoded error messages in TaskCreationWizard with translation keys - Replace hardcoded error messages in TaskEditDialog with translation keys - Use translated default placeholder in TaskFormFields - Internationalize classification dropdown labels (category, priority, complexity, impact) using translation keys instead of hardcoded constants - Add errorMessages parameter to useImageUpload hook for i18n support - Pass translated error messages from TaskFormFields to useImageUpload Logic Bug Fix: - Fix image removal persistence in TaskEditDialog - always set attachedImages to persist removal when all images are deleted (was only set when length > 0) Translation Updates: - Add all missing translation keys to en/tasks.json and fr/tasks.json: - form.errors.* (descriptionRequired, maxImagesReached, etc.) - form.descriptionPlaceholder - form.classification.values.* (all classification option labels) - wizard.descriptionPlaceholder, wizard.errors.* - edit.errors.* Other: - Log image processing errors to console for debugging (CMT-001) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: address memory leak and performance issues in task form components - Add isMounted flag to useEffect in TaskCreationWizard to prevent state updates after component unmount (CMT-QUALITY-001) - Wrap errorMessages merge in useMemo in useImageUpload to prevent unnecessary useCallback invalidation on re-renders (CMT-PERF-001) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: include phaseModels and phaseThinking in hasChanges check TaskEditDialog's hasChanges logic was missing phaseModels and phaseThinking comparisons, which could cause silent data loss when users only modified phase configuration without changing other fields. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: preserve phaseModels and phaseThinking when editing non-autoProfile tasks When editing a task with custom model/thinkingLevel that isn't an autoProfile, the dialog was resetting phaseModels and phaseThinking to defaults instead of preserving the task's actual values from metadata. This could cause data loss. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: Claude Opus 4.5 <[email protected]>
…l sizing
Create shared components to unify TaskCreationWizard, TaskEditDialog, and TaskDetailModal with consistent full-height modal sizing.
New shared components in task-form/:
Benefits:
🤖 Generated with Claude Code
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Related Issue
Closes #
Type of Change
Area
Commit Message Format
Follow conventional commits:
<type>: <subject>Types: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemChecklist
developbranchCI/Testing Requirements
Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: Yes / No
Details:
Summary by CodeRabbit
New Features
Refactor
New UI
Localization
Notes
✏️ Tip: You can customize this high-level summary in your review settings.